Opened 9 years ago

Closed 8 years ago

Last modified 7 years ago

#3363 closed defect (fixed)

Evaluation of parameter args of external objects

Reported by: rfranke Owned by: rfranke
Priority: high Milestone:
Component: Frontend Version: trunk
Keywords: Cc: adrpo

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 follow-up: Changed 9 years ago by 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.

comment:2 Changed 9 years ago by rfranke

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 Changed 9 years ago by rfranke

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 Changed 9 years ago by rfranke

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?

comment:5 in reply to: ↑ 1 Changed 9 years ago by henrikt

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 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.3 to 1.9.4

Moved to new milestone 1.9.4

comment:7 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.4 to 1.9.5

Milestone pushed to 1.9.5

comment:8 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.5 to 1.10.0

Milestone renamed

comment:9 Changed 8 years ago by rfranke

  • Cc adrpo 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 Changed 8 years ago by sjoelund.se

  • Cc adrpo 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 Changed 8 years ago by rfranke

  • Cc adrpo 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 Changed 8 years ago by adrpo

@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 Changed 8 years ago by sjoelund.se

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

comment:14 Changed 8 years ago by adrpo

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

comment:15 Changed 8 years ago by rfranke

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

comment:16 Changed 8 years ago by adrpo

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

comment:17 Changed 8 years ago by sjoelund.se

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

comment:18 Changed 8 years ago by adrpo

@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 Changed 8 years ago by rfranke

  • Owner changed from somebody to rfranke
  • Status changed from new to accepted

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 8 years ago by rfranke (previous) (diff)

comment:20 Changed 8 years ago by rfranke

  • Resolution set to fixed
  • Status changed from accepted to closed

comment:21 Changed 7 years ago by sjoelund.se

  • Milestone 1.10.0 deleted

Milestone deleted

Note: See TracTickets for help on using tickets.