Opened 8 years ago

Closed 6 years ago

Last modified 6 years ago

#4134 closed defect (fixed)

The uses annotation is not interpreted according to the Modelica specification

Reported by: Francesco Casella Owned by: somebody
Priority: blocker Milestone: 2.0.0
Component: Frontend Version:
Keywords: Cc: Martin Sjölund

Description

Consider the following test package:

package A
  model B
    Real x;
    function f
      input Real x;
      output Real y;
    algorithm
      y:= 2*x;
      annotation(uses(Modelica(version="3.1")));
    end f;
  equation
    x = f(time);
    annotation(uses(Modelica(version="3.2.1")));
  end B;
  annotation(uses(Modelica(version="3.2.2")));
end A;

checkModel(A.B); getErrorString();
simulate(A.B); getErrorString();

OMC loads the Modelica Standard Library 3.2.1, with its dependencies. This is wrong, as the Modelica specification (since 2.2) states that

In a top-level class, the version number and the dependency to earlier versions of this class are defined using one or more of the following annotations

Therefore, in the above example, the correct version to be loaded should be 3.2.2.

I would also strongly suggest that the compiler issued warnings whenever it encounters version, conversion, and uses annotations in places others than a top-level class. In this case, I would expect something like:

Warning: A.mo line 9: incorrect uses annotation defined in non top-level class
Warning: A.mo line 12: incorrect uses annotation defined in non top-level-class

Change History (12)

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

Milestone: 1.10.01.11.0

Ticket retargeted after milestone closed

comment:2 by Francesco Casella, 8 years ago

Milestone: 1.11.02.0.0
Priority: highblocker

comment:3 by Francesco Casella, 6 years ago

Component: FrontendNew Instantiation
Owner: changed from somebody to Per Östlund

comment:4 by Per Östlund, 6 years ago

Cc: Martin Sjölund added
Component: New InstantiationFrontend
Owner: changed from Per Östlund to somebody

This isn't a NF issue as such, loading packages based on uses annotations happens before the NF.

in reply to:  4 ; comment:5 by Francesco Casella, 6 years ago

Replying to perost:

This isn't a NF issue as such, loading packages based on uses annotations happens before the NF.

This sounds to me as "happens before the big-bang" :)

Is there a definite module before the NF? Should we give it a Component name on trac and a responsible person?

in reply to:  5 ; comment:6 by Per Östlund, 6 years ago

Replying to casella:

Replying to perost:

This isn't a NF issue as such, loading packages based on uses annotations happens before the NF.

This sounds to me as "happens before the big-bang" :)

Is there a definite module before the NF? Should we give it a Component name on trac and a responsible person?

Before the NF there's the parser, and then the Absyn->SCode phase. They're technically part of the frontend (although the parser has its own Trac component), but the OF and the NF only branch off after the Absyn->SCode phase.

But no matter, I guess I'll just fix this. A quick look at the libraries we bundle reveals that there doesn't seem to be any significant misuse of uses on non-toplevel classes, so it's probably fine to change it.

in reply to:  6 ; comment:7 by Francesco Casella, 6 years ago

Replying to perost:

Before the NF there's the parser, and then the Absyn->SCode phase. They're technically part of the frontend (although the parser has its own Trac component), but the OF and the NF only branch off after the Absyn->SCode phase.

But no matter, I guess I'll just fix this.

Thanks! I guess Adrian currently has other priorities :)

A quick look at the libraries we bundle reveals that there doesn't seem to be any significant misuse of uses on non-toplevel classes, so it's probably fine to change it.

Are you making it an error, a warning, or simply ignoring it?

Also, looking at the regression testing report of your PR, I see messages as

"Warning: Requested package Modelica of version 3.2.2, but this package was already loaded with version 3.2.3. You might experience problems if these versions are incompatible.

According to ticket:5425#comment:6, wasn't this expected to be an OMEdit-only problem?

in reply to:  7 ; comment:8 by Per Östlund, 6 years ago

Replying to casella:

Replying to perost:

A quick look at the libraries we bundle reveals that there doesn't seem to be any significant misuse of uses on non-toplevel classes, so it's probably fine to change it.

Are you making it an error, a warning, or simply ignoring it?

I'm changing it so that we check the uses-annotations on the top-level classes, instead of on the class we're instantiating like we used to do. So uses-annotations on non top-level classes will be ignored.

Also, looking at the regression testing report of your PR, I see messages as

"Warning: Requested package Modelica of version 3.2.2, but this package was already loaded with version 3.2.3. You might experience problems if these versions are incompatible.

According to ticket:5425#comment:6, wasn't this expected to be an OMEdit-only problem?

The test case manually loads "Modelica 3.2", which currently resolves to 3.2.3. So it's doing the same thing that OMEdit does: it preloads a version of the MSL before calling simulate. In this case that's unnecessary, since the package it loads obviously has uses-annotations.

in reply to:  8 comment:9 by Francesco Casella, 6 years ago

Replying to perost:

I'm changing it so that we check the uses-annotations on the top-level classes, instead of on the class we're instantiating like we used to do. So uses-annotations on non top-level classes will be ignored.

OK. It would be nice to issue warnings for all the non-top level classes suggesting to remove them, but I guess we can live without this for the time being. The currently proposed solution is 100% compliant with the spec, so it's just fine.

According to ticket:5425#comment:6, wasn't this expected to be an OMEdit-only problem?

The test case manually loads "Modelica 3.2", which currently resolves to 3.2.3.

OK. I didn't know we were resolving Modelica to 3.2.3, but it seems a very good idea, since all versions of MSL prior to 3.2.1 were completely broken as far as compliance to the standard was concerned.

So it's doing the same thing that OMEdit does: it preloads a version of the MSL before calling simulate. In this case that's unnecessary, since the package it loads obviously has uses-annotations.

I see. Easy fix then :)

comment:10 by Per Östlund, 6 years ago

Resolution: fixed
Status: newclosed

Fixed in 5ef43cc.

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

OpenModelica assumes some part of semantic versioning. So if you ask for "3.2.1" and it doesn't exist, it assumes 3.2.2 is a good substitute. It does not assume if you ask for 3.2.2, 3.2.1 works though (as semantic versioning would allow).

If we had support for conversion scripts, that would help slightly to make sure.

Last edited 6 years ago by Martin Sjölund (previous) (diff)

in reply to:  11 comment:12 by Francesco Casella, 6 years ago

Replying to sjoelund.se:

OpenModelica assumes some part of semantic versioning.

OK, good.

Then it should probably not complain if I load a package using MSL 3.2.2 when I have MSL 3.2.3 loaded already. At least not in this way:

Scripting Warning: Requested package Modelica of version 3.2.2, but this package was already loaded with version 3.2.3. You might experience problems if these versions are incompatible.

See #5427.

Note: See TracTickets for help on using tickets.