Opened 10 years ago

Closed 9 years ago

Last modified 7 years ago

#3363 closed defect (fixed)

Evaluation of parameter args of external objects

Reported by: Rüdiger Franke Owned by: Rüdiger Franke
Priority: high Milestone:
Component: Frontend Version: trunk
Keywords: Cc: Adrian Pop

Description

External objects often require parameters, like a file name or a size.

OpenModelica evaluates such parameters at translation time. This appears not necessary. Can't for instance the count parameter of the following Test model be kept during translation?

package ExtObj
  model MyExternalObject
    extends ExternalObject;
    function constructor
      input Integer count;
      output MyExternalObject ptr;
      external "C" ptr = my_malloc(count);
      annotation(Include = "static void *my_malloc(int count) {
        return malloc(count);}");
    end constructor;

    function destructor
      input MyExternalObject ptr;
      external "C" free(ptr);
    end destructor;
  end MyExternalObject;

  model Test
    parameter Integer count = 10;
    MyExternalObject ptr = MyExternalObject(count);
  end Test;
end ExtObj;

The desired behavior can be achieved, but at the price of ugly code that hardly improves anything:

package ExtObj2
  model MyExternalWorkaround
    extends ExternalObject;
    function constructor
      output MyExternalWorkaround pptr;
      external "C" pptr = my_malloc();
      annotation(Include = "static void *my_malloc() {
        return calloc(1, sizeof(void*));}");
    end constructor;

    function destructor
      input MyExternalObject pptr;
      external "C" my_free(pptr);
      annotation(Include = "static void my_free(void *pptr) {
        if (*(void **)pptr != NULL) free(*(void **)pptr);
        free(pptr);}");
    end destructor;
  end MyExternalWorkaround;

  function create
    input MyExternalWorkaround pptr;
    input Integer count;
    output Integer dummy;
    external "C" dummy = my_create(pptr, count);
    annotation(Include = "static int my_create(void *pptr, int count) {
      if (*(void **)pptr == NULL) *(void **)pptr = malloc(count);
      return count;}");
  end create;

  model TestWorkaround
    parameter Integer count = 10;
    MyExternalWorkaround pptr = MyExternalWorkaround();
    Integer dummyForEquationSorting = create(pptr, count);
  end TestWorkaround;
end ExtObj2;

Change History (21)

comment:1 by Martin Sjölund, 9 years ago

The problem is that external objects are constructed before the initialization step (because they are constructed only once, and so cannot be part of anything that iterates to find a solution). As such, OMC tries to evaluate the constructor's inputs as if they were constants.

comment:2 by Rüdiger Franke, 9 years ago

It's too bad if parameters passed to external objects are pre-evaluated, just because they might be part of some iteration. Couldn't such parameters somehow be marked in the front-end as "must be explicitly known"? It could be checked during generation of initialization code that this is fulfilled.

comment:3 by Rüdiger Franke, 9 years ago

Also here something has changed (cf. #3294). The parameter is kept with

   translateModelFMU(ExtObj.Test)

It is still is pre-evaluated with

   translateModel(ExtObj.Test)

What is the rationale behind?

comment:4 by Rüdiger Franke, 9 years ago

My observation regarding translateModel vs. translateModelFMU was wrong. But ExtObj.Test.count is kept if declared as input -- 6 weeks ago omc raised a warning in this case. Why not keep parameters as well?

in reply to:  1 comment:5 by Henrik Tidefelt, 9 years ago

Replying to sjoelund.se:

The problem is that external objects are constructed before the initialization step (because they are constructed only once, and so cannot be part of anything that iterates to find a solution). As such, OMC tries to evaluate the constructor's inputs as if they were constants.

Often, the constructor argument is a parameter, and that constructor call cannot be part of any iteration does not mean that it is not meaningful to change the parameter value with the variability of a non-structural parameter.

comment:6 by Martin Sjölund, 9 years ago

Milestone: 1.9.31.9.4

Moved to new milestone 1.9.4

comment:7 by Martin Sjölund, 9 years ago

Milestone: 1.9.41.9.5

Milestone pushed to 1.9.5

comment:8 by Martin Sjölund, 9 years ago

Milestone: 1.9.51.10.0

Milestone renamed

comment:9 by Rüdiger Franke, 9 years ago

Cc: Adrian Pop added

a7a4dbee/OMCompiler keeps parameters. The result of the nightly tests is not so bad. ExtObj.Test works as intended (count can be changed after translation and this takes effect on the external object).

Some models fail though because they initialize external objects with bound parameters that are initialized after the creation of the external object. See e.g.:

ModelicaTest.Tables.CombiTable2D.Test22
ModelicaTest.Tables.CombiTimeTable.Test68

This example reproduces the problem:

package ExtObj3
  model MyExternalObject
    extends ExternalObject;
    function constructor
      input Integer count;
      output MyExternalObject ptr;
      external "C" ptr = my_malloc(count);
      annotation(Include = "static void *my_malloc(int count) {
        printf(\"count = %d\\n\", count);
        return malloc(count);}");
    end constructor;

    function destructor
      input MyExternalObject ptr;
      external "C" free(ptr);
    end destructor;
  end MyExternalObject;

  model Test
    parameter Integer count = 10;
    parameter Integer count2 = count;
    MyExternalObject ptr = MyExternalObject(count2);
  end Test;
end ExtObj3;

Before a7a4dbee/OMCompiler, count2 was evaluated to constant 10. Nevertheless the translated model still offered to change the parameter count after translation, without taking effect on the creation of the external object.

Now count2 is uninitialized for the construction of the external object. This makes the problem visible in the nightly coverage tests. The workaround would be to explicitly add annotation(Evaluate=true) to count2.

The solution would be to not evaluate, but eliminate the bound parameter count2, such that count can still be changed after model translation and also takes effect on the creation of the external object.

How can count2 be eliminated instead of evaluated?

comment:10 by Martin Sjölund, 9 years ago

Cc: Adrian Pop removed

The a9f3edc2743b0fed87bd2b69757e2433c0f3436b/OpenModelica-testsuite together with 098036e9bb81f7b27acd7290a5967d01f39f6a68/OpenModelica-testsuite seems to have break Buildings models (now says usertab not implemented, which is a sign of sending in "NoName" for some reason).

comment:11 by Rüdiger Franke, 9 years ago

Cc: Adrian Pop added

With d341b73/OMCompiler the nightly tests should work as before. The new Static.evalExternalObjectInput attempts to identify and keep free parameters. The initial ExtObj example works now.

Unfortunately this does not work for array inputs, because the frontend seems to replace the cref for the array with an array of crefs for each element. Can the frontend be changed to keep the array cref, e.g. counts, instead of {counts[1],counts[2],counts[3],counts[4]} in this case?

This should also help to generate more efficient code for models with arrays.

See e.g.:

package ExtObj4
  model MyExternalObject
    extends ExternalObject;
    function constructor
      input Integer[:] counts;
      output MyExternalObject ptr;
    protected
      Integer count = sum(counts);
      external "C" ptr = my_malloc(count);
      annotation(Include = "static void *my_malloc(int count) {
        printf(\"count = %d\\n\", count);
        return malloc(count);}");
    end constructor;

    function destructor
      input MyExternalObject ptr;
      external "C" free(ptr);
    end destructor;
  end MyExternalObject;

  model Test
    parameter Integer[:] counts = {1, 2, 3, 4};
    MyExternalObject ptr = MyExternalObject(counts);
  end Test;
end ExtObj4;

comment:12 by Adrian Pop, 9 years ago

@rfranke: is not possible to keep the arrays unexpanded with the current front-end without major changes. However, I think there is a function somewhere that will implode the array back if needed (I'll search for it).

comment:13 by Martin Sjölund, 9 years ago

There is a vectorization limit that can be used to change where to split the arrays as well.

comment:14 by Adrian Pop, 9 years ago

@sjoelund.se: true, but I think many things will break if we change that number.

comment:15 by Rüdiger Franke, 9 years ago

Thank's for the hints. I think I will iterate over the array elements to check them.

comment:16 by Adrian Pop, 9 years ago

@rfranke: you might be able to use: ComponentReference.implode to make the array back into a cref.

comment:17 by Martin Sjölund, 9 years ago

Nah. Implode is for making a qualified CREF out of a list of CREF_IDENT. ({a,b,c} -> a.b.c).

comment:18 by Adrian Pop, 9 years ago

@sjoelund.se: you're right. It seems we only have cref -> array vectorization and not the other way around (or i couldn't find one).

comment:19 by Rüdiger Franke, 9 years ago

Owner: changed from somebody to Rüdiger Franke
Status: newaccepted

See 0182b27/OMCompiler -- since I've learnt that one should use for loops instead of meta-functional, it's only Hudson's merge conflicts and other asap tests left to slow down commits ;-)

This should solve the ticket in a reasonable way.

Last edited 9 years ago by Rüdiger Franke (previous) (diff)

comment:20 by Rüdiger Franke, 9 years ago

Resolution: fixed
Status: acceptedclosed

comment:21 by Martin Sjölund, 7 years ago

Milestone: 1.10.0

Milestone deleted

Note: See TracTickets for help on using tickets.