#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 , 8 years ago
Milestone: | 1.10.0 → 1.11.0 |
---|
comment:2 by , 8 years ago
Milestone: | 1.11.0 → 2.0.0 |
---|---|
Priority: | high → blocker |
comment:3 by , 6 years ago
Component: | Frontend → New Instantiation |
---|---|
Owner: | changed from | to
follow-up: 5 comment:4 by , 6 years ago
Cc: | added |
---|---|
Component: | New Instantiation → Frontend |
Owner: | changed from | to
This isn't a NF issue as such, loading packages based on uses
annotations happens before the NF.
follow-up: 6 comment:5 by , 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?
follow-up: 7 comment:6 by , 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.
follow-up: 8 comment:7 by , 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?
follow-up: 9 comment:8 by , 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.
comment:9 by , 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 :)
follow-up: 12 comment:11 by , 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.
comment:12 by , 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.
Ticket retargeted after milestone closed