simulate(..) returns record instead of string

[Bernhard Thiele]

According the its signature the MOS scripting function simulate(..) should return a String. However, (?sometimes?) a record is returned.

Consider following MOS script:

loadModel(Modelica);

res := simulate(Modelica.Blocks.Examples.PID_Controller);
print("A");
print(res); // No error, but this line will be blank?
print("B");

if true then
  res := simulate(Modelica.Blocks.Examples.PID_Controller);
  print(res); // Throws an error that a record is passed, but a String is expected
else
  print("Just to have an else");
end if;

The first print doesn't work (but throws no error), the second print fails with a type error (expected String, but got a record).

How is the return value of simulate(..) supposed to work?

[Bernhard Thiele]

The mos script calling simulate(..)

[anonymous]

It is supposed to be a record that is passed. If the ModelicaBuiltin.mo says otherwise, that is an error.

[Bernhard Thiele]

OK. I just finished a commit that would change the return value to a String (​https://github.com/OpenModelica/OMCompiler/pull/953).

However, I'm also fine with a record, which would mean that the signature should be fixed.

Some of the record fields are created dynamically in CevalScriptBackend.createSimulationResult(..). If one was to fix the signature, I suppose one will need a static record type for the return value?

[Martin Sjölund]

I think the full record should always be created, but when doing ValuesUtil.valString to show the value in the mos-script it should filter out the fields that are not testsuite-friendly.

[Bernhard Thiele]

Replying to sjoelund.se:

I think the full record should always be created, but when doing ValuesUtil.valString to show the value in the mos-script it should filter out the fields that are not testsuite-friendly.

I'm not sure what you mean. There are all these time* entries which are not testsuite-friendly. However, they are probably okay as user information after a simulation. Maybe testing should use a less verbose call to simulate(..), but giving some more information as default output is fine.

The problem of changing the simulate(..) output is that all test-cases would need to be adapted if the "default" output changes only a tiny bit from the current output. Running the test suite with my "patch" results in more than 50% test case failure...

[Martin Sjölund]

I just think it should be fine if the value internally has all of these things, but when printing things for the testsuite they are simply not shown. So the time* entries could be accessed using simRes.timeXXX, but timeXXX would not be printed when displaying the record SimulationResult ....

[Bernhard Thiele]

Just realized that in CevalScriptBackend.createSimulationResult there is already some logic which seems to detect whether it is a test run and in that case it won't add all the time* fields.

I suppose that if a record is returned from a function in the interactive environment it should be displayed without keeping track of special cases where the record came from, e.g., display the record differently if it is the result of a simulate(..) call. However, you seem to propose a special case treatment for simulation records?

Also, if we return a record at least the signature of ModelicaBuiltin.simulate(..) should be fixed to return a record and not a string. However, in that case we need to declare the record type first. And this amounts to decide which fields are in the record. In line 819 of ModelicaBuiltin.mo there is a SimulationResult declaration, but it is commented out.

So, what to do with it?