Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CheckContactCalculation and other Roads.Internal models fail in OpenModelica #18

Closed
dariomangoni opened this issue May 22, 2018 · 13 comments
Labels
example Issue only addresses example(s) wontfix Issue that will not be fixed

Comments

@dariomangoni
Copy link
Contributor

dariomangoni commented May 22, 2018

Models contained inside Roads.Internal suggest that something is not working properly.
Any Check_____ function report the following error:

[1] 11:08:04 Translation Warning
[VehicleInterfaces.Roads: 479:7-479:57]: No corresponding 'inner' declaration found for component .VehicleInterfaces.Roads.Interfaces.Base road.roadShape.road declared as 'outer '.
  The existing 'inner' components are:
    .Modelica.Mechanics.MultiBody.World$world world; defined in scope: VehicleInterfaces.Roads.Internal.CheckContactCalculation. Referenced by 'outer' components: {road.world}
  Check if you have not misspelled the 'outer' component name.
  Please declare an 'inner' component with the same name in the top scope.
  Continuing flattening by only considering the 'outer' component declaration.

[2] 11:08:04 Translation Error
[VehicleInterfaces.Roads: 173:5-231:13]: Illegal to instantiate partial class Base.

[3] 11:08:04 Translation Error
Error occurred while flattening model VehicleInterfaces.Roads.Internal.CheckContactCalculation

It seems that those components (such as DummyTyre) that require an inner declaration of road are not connecting to the inner declaration that is actually found in Check______ models.

But here is the thing: I suppose that probably the error is in Internal.VisualizeSimpleRoads @480 at the line outer VehicleInterfaces.Roads.Interfaces.Base road;.

Commenting it, everything works fine.

I didn't make a Pull Request, because I'm not sure of the solution.

@tobolar
Copy link
Contributor

tobolar commented Jun 5, 2018

I cannot find any problem when checking in Dymola.
Even your log message looks strange.

[1] 11:08:04 Translation Warning
[VehicleInterfaces.Roads: 479:7-479:57]: No corresponding 'inner' declaration found for component .VehicleInterfaces.Roads.Interfaces.Base road.roadShape.road declared as 'outer '.
The existing 'inner' components are:
.Modelica.Mechanics.MultiBody.World$world world; defined in scope: VehicleInterfaces.Roads.Internal.CheckContactCalculation. Referenced by 'outer' components: {road.world}

Actually, there are two "inner" components in VehicleInterfaces.Roads.Internal.CheckContactCalculation and not just one:

  • inner VehicleInterfaces.Roads.CircleRoad road
  • inner Modelica.Mechanics.MultiBody.World world

So the issue could be that the inner road is looked for in the road component itself. But I don't believe this is a bug.
So the question is what is your tool for checking.

@dariomangoni
Copy link
Contributor Author

dariomangoni commented Jun 5, 2018

Yes, you are right.
There are two inner components also for me. No doubt about it.
The problem is that, at least with OpenModelica, the compiler throws errors.

My guess (and probably we are saying the same thing):

  • the inner component at the top level called road is of type CircleRoad
  • CircleRoad contains VisualizeSimpleRoads
  • VisualizeSimpleRoads has an outer declaration of road, thus it means that VisualizeSimpleRoads is requiring a component called road defined as inner (i.e. the top-level road)

So there is a sort of loop.
VisualizeSimpleRoads is requiring an inner road component and at the same time is part of it.
It's just a guess.

Anyway, in OpenModelica CheckContactCalculation does not work. I don't know if declaring a component as 'outer' and being at the same time part of the 'inner' instance is allowed, but it would sound strange to me.

BTW, the error log reports that it finds only one 'inner' declaration because the other 'inner' declaration (i.e. road) is actually what is causing the issue! That's probably why it is not reported in the list of 'inner'-declared components.

The second error, instead, I think is generated because the compiler is disregarding the 'outer' attribute of the road variable contained inside VisualizeSimpleRoads and, since it is declared as a Base class, it's throwing an error. But this is not the main cause, it is just a compiler rollback!

2018-06-05 10_09_55-window

@tobolar
Copy link
Contributor

tobolar commented Jun 5, 2018

I guess your analysis is right.

I have looked at Modelica Language Specification for inner/outer definition. It is defined in Section 5.4 Instance Hierarchy Name Lookup of Inner Declarations and differs depending on the Specification version.
I haven't found (without guaranty) any comment on "looping" inner/outer. The current Specification (https://modelica.org/documents/ModelicaSpec33Revision1.pdf) only states that

An outer element component may be of a partial class [but the referenced inner component must be of a non-partial class].

I guess this could be the point. Can you consult this with OpenModelica's team?

@dariomangoni
Copy link
Contributor Author

Unfortunately I don't have any contact with OpenModelica developers.
But honestly, I think that this loop can and should be avoided in any case. I don't think that a visualizer like VisualizeSimpleRoads should have this outer component at all.

I would suggest to modify VisualizeSimpleRoads in this way: the road variable is used only to pass the position function to the underlying Modelica.Mechanics.MultiBody.Visualizers.Advanced.Surface, through the surfaceCharacteristic encapsulated function.
But, as in the Surface model itself, this input can be provided by the model that instantiate VisualizeSimpleRoads (i.e. FlatRoad) without any outer road component.

In short:

VisualizeSimpleRoads should have an input, namely
input VehicleInterfaces.Roads.Interfaces.positionBase position; that will be passed as input to roadSurfaceCharacteristic that is in turn used to extend Modelica.Mechanics.MultiBody.Visualizers.Advanced.Surface;

Now: FlatRoad, while instantiating VisualizeSimpleRoads, should now provide not only ns, nw and so on... but also a positionBase function, that in this case should be linePosition.

However, I tried this solution and it does not work as it is... In particular about the Roads interface class...
I will probably open another issue about this.

@dietmarw
Copy link
Member

dietmarw commented Jun 6, 2018

Unfortunately I don't have any contact with OpenModelica developers.

This can be easily helped:

@dariomangoni
Copy link
Contributor Author

dariomangoni commented Jun 6, 2018

Well...
Point 1: if a convoluted code can be avoided, I think it should be avoided; otherwise, you are free to say "hey, my library is working with Dymola, thus I'm fine even if it is not working with OpenModelica"
Point 2: I can write on that forum for you and I will, but I think VehicleInterfaces developers may write as well and it will be much better since they communicate with no intermediate person; it is not for laziness (I already did my best trying to offer you my modest advice) and I will write.
Point 3: I never considered the forum as an effective way for consulting OpenModelica's team: have you ever tried to write in that forum? I did, and I never received a reply. The same for the issue tracker.
Maybe this time will go better? Let's see...

Opened post in OpenModelica forum:
https://openmodelica.org/forum/default-topic/2500-a-inner-component-may-have-an-outer-declaration-to-itself#top

@perost
Copy link

perost commented Jun 6, 2018

@dariomangoni OpenModelica dev here, I'll just answer here instead of the forum since the main discussion is here.

You're right, it's not clear in the specification whether that's allowed or not. But it also doesn't seem to be the actual issue, because OMC currently goes into an instantiation loop for your example model. That is to say, if the model actually looked like that you wouldn't get the error you're getting, the compiler would just crash.

The frontend in the current version of OMC has many issues with inner/outer though, so this looks like just another bug in it. We've been working on replacing the old frontend with a completely new one (the old one might charitably be called "hard to maintain"), but it's still work in progress. But using the new frontend (with the command line option -d=newInst) the VehicleInterfaces.Roads.Internal.CheckContactCalculation model flattens just fine in the latest nightly build, with no inner/outer errors or any other issues.

But as I said it's not quite ready yet, and the model currently fails to simulate because we're still missing support for overconstrained connections. We're working on that and hope to have it in any day now. So that particular model should probably simulate soon with OpenModelica, although I can't say how well the rest of the library will work. Only about 1/3 of the MSL simulate correctly using the new frontend right now, but it's rapidly improving.

@tobolar
Copy link
Contributor

tobolar commented Jun 6, 2018

VisualizeSimpleRoads should have an input, namely...

Propagating this function makes things complicated. Inner/outer is in contrast nice if it works properly.

@harmanpa harmanpa added bug Critical/severe issue example Issue only addresses example(s) labels Jun 7, 2018
@tobolar tobolar added the wontfix Issue that will not be fixed label Jun 8, 2018
@tobolar tobolar closed this as completed Feb 4, 2019
@perost
Copy link

perost commented Feb 4, 2019

I had a look at the current state of the model in OpenModelica, and noticed it no longer flattened with the new frontend anymore. It was just a minor issue that I've fixed now, and I added the VehiceInterfaces library to our automatic coverage testing to avoid further regressions. The state of the library should be viewable at https://libraries.openmodelica.org/branches/newInst/VehicleInterfaces/VehicleInterfaces.html in a couple of hours if everything goes smoothly.

Using the new frontend (compiler flag -d=newInst), the model now gets all the way to the simulation stage where it segfaults because the generated C-code tries to copy data into an array literal. I'll look into it, it shouldn't be too hard to fix.

@dariomangoni dariomangoni changed the title CheckContactCalculation and other Roads.Internal models fail CheckContactCalculation and other Roads.Internal models fail in OpenModelica Feb 18, 2020
@tobolar
Copy link
Contributor

tobolar commented Mar 6, 2020

I believe PR #47 may be a fesible solution but still prefere the existing implementation. Even the comment #18 (comment) above states this shall be working now in OM as well.
Optionally, this issue could be solved by clarification of Modelica specification but I'm not the responsible one.

@tobolar tobolar reopened this Mar 6, 2020
@dariomangoni
Copy link
Contributor Author

Yes, in OMC it's working but only with the new frontend, that turns out to be quite bugged for other perspectives.

If I look at the MSL, for example in Modelica.Mechanics.MultiBody.Visualizers.Advanced.Surface, I think there is no outer declaration of word. Everything is passed (liked the function surfaceCharacteristic as a functional input.
Why do you think that the current implementation is better? Doesn't it put more stress on the compiler, doesn't it introduce a not-so-nice loop that could be easily avoided?

Again, you all have a wider experience than me. Mine it's just an idea.

@tobolar
Copy link
Contributor

tobolar commented Apr 17, 2020

The existing implementation is definitely more simple to use - for exactly there is no additional input "position" the user has to care about. And as long as it is conform to Modelica Specification, there is no reason to prefer #47 over it.

@tobolar tobolar removed the bug Critical/severe issue label Apr 17, 2020
@tobolar tobolar closed this as completed Apr 17, 2020
@tobolar
Copy link
Contributor

tobolar commented Apr 30, 2020

Just for the sake of completeness: The currently implemented solution was introduced by @harmanpa in cb28925.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
example Issue only addresses example(s) wontfix Issue that will not be fixed
Projects
None yet
Development

No branches or pull requests

5 participants