OMEdit crashes(Windows)/does nothing(Linux) when instance has same name as package

[dietmarw]

In the attached small package the model TestGen contains an instance of the model EMF which is named "DiscGenerator" just like the package. This seems to cause all kinds of problems which crashes OMEdit on Windows and throws errors in Linux.

Changing the instance name to something else like "discGenerator" solves the problem.

(Mind the models in the example package contain other modelling errors, but are not of interest here. I wanted to keep the example as original as possible.)

[casella]

@dietmarw, I tried to run this model in OMEdit with -d=newInst and I got this error message:

[2] 20:47:07 Translation Error
[DiscGenerator: 78:3-79:115]: Class name 'DiscGenerator.EMF' was found via a
component (only component and function call names may be accessed in this way).

which I guess is correct, you should not use the same name for an instance and for a class (the top package, in this case).

So, this is already fixed for 2.0.0, when the new front-end will become the default one.

[dietmarw]

@casella
Allow me to disagree. First having a component "Foo.Bar.Baz" and then using that component in a model "Foo.Bar.Test" where it is called "Foo" is perfectly legal. So it should not cause an error.

Also saying this is fixed in 2.0.0 and close it for 1.14.0 would be acceptable if it would not CRASH OMEdit. Something that if know really needs to be fixed. I mean at least the crash. So please consider to fix at least that part.

[casella]

Replying to dietmarw:

@casella
Allow me to disagree. First having a component "Foo.Bar.Baz" and then using that component in a model "Foo.Bar.Test" where it is called "Foo" is perfectly legal. So it should not cause an error.

OK, I was too fast. Personally, I always use different names for components and classes to avoid weird side effects, so I'm not really sure what the language specification exactly allows from this point of view.

@perost, can you please comment on that?

Also saying this is fixed in 2.0.0 and close it for 1.14.0 would be acceptable if it would not CRASH OMEdit. Something that if know really needs to be fixed. I mean at least the crash. So please consider to fix at least that part.

You right in principle. In practice, we have a very large number of open issues in OMC, many of which cause crashes, and we have to prioritize, given our very limited resources. As a general policy, we are no longer fixing issues in the old front end, which is going to be retired in a few months from now, and instead employing all available resources (mainly @perost) trying to get the new front-end up to speed as soon as possible.

I understand this policy may be questionable - in fact, I had long discussions about this with my office mate. I think it's the best one from a strategic point of view, since the new front end is the only way to solve a large number of very serious issues that have been known for years (see, e.g., #2858, and more in general this list), and in general to achieve performance comparable to commercial tools.

Please note that the coverage with the new front end is already quite high: with the MSL, it is actually aready better than the old one, and with most library is quite close. So, there are good chances (in general) that your model works with it. In that case, you can already use it, without waiting for 2.0.0: just turn on the -d=newInst compiler flag, or activate it with Simulation Setup | Translation phase | Enable experimental new instantiation phase.

As to the fact that the new front end actually does not process your model, I'll leave it to @perost (see above).

[perost]

Replying to casella:

@perost, can you please comment on that?

The issue with regards to the NF is that DiscGenerator in the TestGen model refers to the component of the same name, since the name lookup will look in the TestGen scope before looking in enclosing scopes (5.3.1). So DiscGenerator.EMF mean "look up EMF in the local component DiscGenerator", which is not valid since the lookup rules (5.3.2) forbids looking up a class inside a component except when used as a function call. Hence why you get the error that you get when using the NF.

If you want to refer to the DiscGenerator package in the scope of TestGen while having elements of the same name in that scope you will have to be more specific, either by fully qualifying the path or using a renaming import. So as far as the NF is concerned there is no issue, the model is not valid.

This does not mean that it should cause OMEdit to crash of course, but this is due to the old frontend being used by the interactive API (enabling the NF in OMEdit will not change this). For this to be fixed we will have to wait for Adrian's work on using the NF for the interactive API.

[casella]

Replying to perost:

The issue with regards to the NF is that DiscGenerator in the TestGen model refers to the component of the same name, since the name lookup will look in the TestGen scope before looking in enclosing scopes (5.3.1). So DiscGenerator.EMF mean "look up EMF in the local component DiscGenerator", which is not valid since the lookup rules (5.3.2) forbids looking up a class inside a component except when used as a function call. Hence why you get the error that you get when using the NF.

As I understand, this is a very restrictive interpretation of the specification. First you look in the local scope, and since you can't look up a class inside a component except for a function call, you find nothing. So, you continue in the enclosing scopes, until you find the DiscGenerator package name, then successfully find EMF in it. The specification does not say that if you encounter a name in which is invalid in the current scope you should abort the look-up. Why not just continuing it?

This does not mean that it should cause OMEdit to crash of course, but this is due to the old frontend being used by the interactive API (enabling the NF in OMEdit will not change this). For this to be fixed we will have to wait for Adrian's work on using the NF for the interactive API.

OK, so the crash will be automatically avoided in 1.14.0, which will use the NF by default for all the OMEdit operations, as soon as Adrian pushes in his work. No need on him to take specific actions.

[perost]

Replying to casella:

Replying to perost:

The issue with regards to the NF is that DiscGenerator in the TestGen model refers to the component of the same name, since the name lookup will look in the TestGen scope before looking in enclosing scopes (5.3.1). So DiscGenerator.EMF mean "look up EMF in the local component DiscGenerator", which is not valid since the lookup rules (5.3.2) forbids looking up a class inside a component except when used as a function call. Hence why you get the error that you get when using the NF.

As I understand, this is a very restrictive interpretation of the specification. First you look in the local scope, and since you can't look up a class inside a component except for a function call, you find nothing. So, you continue in the enclosing scopes, until you find the DiscGenerator package name, then successfully find EMF in it. The specification does not say that if you encounter a name in which is invalid in the current scope you should abort the look-up. Why not just continuing it?

That seems incredibly error prone and unlike any programming language I've used. It's also not what Dymola does, i.e. the following model will give an error in Dymola:

package P
  package P
    model M
      Real x;
    end M;
  end P;

  model M2
    Real y;
  end M2;

  model test
    P.M2 m2; // Error: Component type specifier P.M2 not found
  end test;
end P;

I suspect that Dymola might ignore components when looking for a class, but the specification explicitly says that the name lookup is done among all named elements.

[casella]

@dietmarw: case closed?

After all, decades-old Modelica style rules suggest using capital letters for class names and small letters for component names...

[dietmarw]

@casella Don't confuse conventions with rules. If lowercase instance names would be a rule then probably 99% of the libraries would fail on that. Making instances of classes lower case is just a convention (i.e., recommendation) that also I can not always follow for practical reasons)

The crashing issue has been resolved though I still think OMC is doing something wrong with the lookup here.

Following the minimal example I understand that the following will fail:

model test
    P.M2 p; // Error: Component type specifier P.M2 not found
end test;

whilst this will work:

model test
    P.M p; // Success
end test;

BUT this here should work too:

model test
    P.M P; // stack over flow
end test;

and does so in Dymola (hate to bring this up). You can not expect that when one names instances to check for crashes on all levels of the library. The instance name is after all P.test.P and hence should not cause any problems what so ever.

[casella]

@perost, any comment?

[perost]

Replying to dietmarw:

BUT this here should work too:

model test
    P.M P; // stack over flow
end test;

and does so in Dymola (hate to bring this up).

This does not work in Dymola though:

package P
  package P
    model M
      Real x;
    end M;
  end P;

  model M2
    Real y;

    model A
      Real x;
    end A;
  end M2;

  model test
    P.M M2;
    M2 m1;   // Fails due to name lookup finding local component M2.
    M2.A m2; // Also fails.
  end test;
end P;

My point here is that Dymola seems to ignore the component it's currently looking up the type for, but no other elements. That's rather confusing, and quite probably a bug in Dymola.

[adrpo]

I agree that:

model test
    P.M P; // stack over flow
end test;

should not crash OMEdit.
As to if we should support this or not, is another question.
So you are in the scope of test and you are looking for P, which P should you find?

[dietmarw]

Well the local P first. So if no P can be found inside test then you would move up.

[adrpo]

So local P is the component P. Has type P dot something, looking for P again, which one should you find? :)

[dietmarw]

That test model failing is perfectly understandable and predictable. You can easily take care of. You should not refer to local classes. So:

model test
  P.M M2;
  .P.M2 m1;   // don't reference local instances as classes but point to the absolute path
  M2.A m2; // will always fail since models inside models are encapsulated
   end test;

[dietmarw]

I think I found another related quirk. I described in the small screen cast. The library version I used to demonstrate is this: ​https://github.com/dietmarw/WasteWater/releases/tag/v3.0.0-alpha.2

Here the screen cast: ​https://www.dropbox.com/s/7i1pu67j2iyqw46/2019-09-20_OMCNameLookup.mkv?dl=0

[adeas31]

Replying to dietmarw:

I think I found another related quirk. I described in the small screen cast. The library version I used to demonstrate is this: ​https://github.com/dietmarw/WasteWater/releases/tag/v3.0.0-alpha.2

Here the screen cast: ​https://www.dropbox.com/s/7i1pu67j2iyqw46/2019-09-20_OMCNameLookup.mkv?dl=0

See #5639.
Seems like there are some issues when using flag -d=nfAPI (OMEdit sets it by default). You can disbale it by unchecking Tools->Options->Simulation->Enable new frontend use in the OMC API (faster GUI response).

[dietmarw]

Replying to adeas31:

Seems like there are some issues when using flag -d=nfAPI (OMEdit sets it by default). You can disbale it by unchecking Tools->Options->Simulation->Enable new frontend use in the OMC API (faster GUI response).

Yes that worked. So is there a way to have the old front end by default without users first going into the settings?

[adeas31]

The idea is to use the new frontend :)

[dietmarw]

Which isn't ready yet :-P

[casella]

Well, every time you start OpenModelica, you will have to go through one failed compilation cycle and click on the switch to the old frontend permanently button. No big deal.

I think resetting this setting every time you start a new session is a good idea: you may be using the same OpenModelica installation to work on multiple projects, some of which may be better using the NF and some others using the OF. Always defaulting to NF, seems the right thing to do, since it has the best performance in most cases.

[dietmarw]

It is not a big deal for an experienced user but for new-beginners it is rather off-putting.

[casella]

@dietmarw, I'm not sure I understand your requirement here. The decision was to have the NF and nfAPI as default, because in most cases they perform much better than the old ones. In case you want the other option, there are settings to do so, so you can always do that. We understand this may inconvenience some users, but it will make most users much happier.

In a few more months the NF should be complete, and then we'll also stop inconviencing this smaller user group.

[dietmarw]

@casella From a QA point of view for stable releases you would not activate new features that have shown to break the user experience by default. So normally you would have the old frontend by default and would inform your power users that there is a newer but still unstable frontend available that can be activated (but is not by default).

As far as I understand, this is the case for OMC itself so why treat this in OMEdit differently.

[casella]

Replying to dietmarw:

@casella From a QA point of view for stable releases you would not activate new features that have shown to break the user experience by default. So normally you would have the old frontend by default and would inform your power users that there is a newer but still unstable frontend available that can be activated (but is not by default).

This was the original plan, when we thought we would release 1.14.0 with replaceable support six months ago. In the meantime, the new frontend became a lot better, so the situation changed drastically. To summarize:

• the coverage of MSL with the NF is the same as with the OF, the remaining problems are due to backend/runtime issues
• the user experience with the NF in terms of speed of response is dramatically improved (more than 10x faster flattening, more than 10x faster GUI responsiveness) - this actually means that dealing with non-trivial Fluid or MultiBody models is now possible, while it was not really practical with the OF (more than one minute wait after a mouse action)
• most models that worked with the OF work with the NF; additionally, there are many models that work only with the NF; for example, at Dynamica we only use the NF, because the OF is simply not good enough to deal with our libraries and models
• for the few regressions w.r.t. the old front-end, you are one click away from the use of the old front end, either temporarily or permanently.

If we made the NF an option marketed for "power" or "advanced" users, only a few inner circle daredevils would have tried it out, while the majority of users that would actually get huge advantages by doing so would have continued threading with the OF for one more year. BTW, the OF is no longer maintained since 2018.

We are aware of the inconvenience that we are causing to some users - we tried to minimize that, and we are convinced that the advantages for most users far outweight this.

As far as I understand, this is the case for OMC itself so why treat this in OMEdit differently.

OMC from the command line is different. People who use that interface are expected to be a lot more technically savy, so we can explain them in the release notes that they need -d=newInst to get the advantages of the new frontend.

[anonymous]

Replying to dietmarw:

I think I found another related quirk. I described in the small screen cast. The library version I used to demonstrate is this: ​https://github.com/dietmarw/WasteWater/releases/tag/v3.0.0-alpha.2

Here the screen cast: ​https://www.dropbox.com/s/7i1pu67j2iyqw46/2019-09-20_OMCNameLookup.mkv?dl=0

Maybe you should open a ticket specifically on this?
I suppose it needs to be a blocker for OM 1.14

[casella]

Replying to anonymous:

Replying to dietmarw:
Maybe you should open a ticket specifically on this?

That would be appreciated, this ticket became a bit too cluttered

[casella]

Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2. This issue is rescheduled to 1.16.0

[casella]

Retargeted to 1.17.0 after 1.16.0 release

[casella]

Retargeted to 1.18.0 because of 1.17.0 timed release.

[casella]

Ticket retargeted after milestone closed