Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#4520 closed defect (fixed)

Lookup: how to start from top level

Reported by: m.thorade@… Owned by: somebody
Priority: normal Milestone: 1.12.0
Component: Run-time Version:
Keywords: Cc: adrpo, sjoelund.se

Description

This line fails to work in OpenModelica:
https://github.com/UdK-VPT/BuildingSystems/blob/9c13fdd95c551e974ecd8599d2f3cf06c6962d09/BuildingSystems/Utilities/NcDataReader2/NcDataReader.mo#L5
The model is in the package BuildingSystems.Utilities.NcDataReader2 and tries to import something from another library which happens to have the same name NcDataReader2

The easiest solution would be to rename the package in our library, but are there other solutions?
I already tried
import nc = .NcDataReader2.Functions;
because I thought that was the syntax to tell the tool to start looking on the top level, but that also failed.

Should that be fixed on OpenModelica side, or is our model just wrong? It works as it is in Dymola.

Change History (11)

comment:2 Changed 7 years ago by tbc@…

Subscribe issue

comment:3 Changed 7 years ago by m.thorade@…

This is a better example:
https://libraries.openmodelica.org/branches/master/BuildingSystems/files/BuildingSystems_BuildingSystems.Technologies.SolarThermal.Examples.SingleCollectorTest.err
because it had only this one error.

I renamed the package in our library here:
https://github.com/UdK-VPT/BuildingSystems/pull/80
Lookup then works, but a new error is generated:
Could not find library netcdf in either of:
and a list of directories

comment:4 Changed 7 years ago by perost

  • Cc adrpo sjoelund.se added
  • Component changed from *unknown* to Run-time

Imports are always looked up from the top level, so using a dot at the start of the import path is redundant. It's actually not even allowed by the Modelica syntax, so I have fixed our parser to be more compliant and reject such code.

I have also fixed the lookup issue, which was actually not related to imports at all but a bug in our handling of extends. I don't know what the issue is with not finding the library though, maybe someone else has a better clue about that issue.

comment:5 follow-up: Changed 7 years ago by m.thorade@…

Thanks for clarification & the fix!

So, the issue with not finding the library should become visible here pretty soon:
https://libraries.openmodelica.org/branches/master/BuildingSystems/files/BuildingSystems_BuildingSystems.Technologies.SolarThermal.Examples.SingleCollectorTest.err

The same issue can also be reproduced by running the FullAPI example from the NcDataReade2 library, as reported here:
https://trac.openmodelica.org/OpenModelica/ticket/4356#comment:4
That model used to work in the past, IIRC.

comment:6 in reply to: ↑ 5 Changed 7 years ago by perost

Replying to m.thorade@…:

So, the issue with not finding the library should become visible here pretty soon:
https://libraries.openmodelica.org/branches/master/BuildingSystems/files/BuildingSystems_BuildingSystems.Technologies.SolarThermal.Examples.SingleCollectorTest.err

Yes, my fix is pushed into master, so it should hopefully be visible tomorrow.

comment:7 follow-up: Changed 7 years ago by anonymous

Imports are always looked up from the top level, so using a dot at the start of the import path is redundant. It's actually not even allowed by the Modelica syntax, so I have fixed our parser to be more compliant and reject such code.

Should https://github.com/modelica/Modelica-Compliance/blob/master/ModelicaCompliance/Scoping/NameLookup/Imports/Recursive.mo#L7 be fixed then to point to the recursive import problem instead to the leading dot.

comment:8 in reply to: ↑ 7 Changed 7 years ago by perost

Replying to anonymous:

Imports are always looked up from the top level, so using a dot at the start of the import path is redundant. It's actually not even allowed by the Modelica syntax, so I have fixed our parser to be more compliant and reject such code.

Should https://github.com/modelica/Modelica-Compliance/blob/master/ModelicaCompliance/Scoping/NameLookup/Imports/Recursive.mo#L7 be fixed then to point to the recursive import problem instead to the leading dot.

The syntax for imports doesn't seem to have changed since they were added in Modelica 1.4, and has never allowed a leading dot. So yes, that test case should be fixed.

comment:9 Changed 7 years ago by anonymous

The lookup has been fixed and works as expected and the compliance model has been updated, so I think this ticket can be closed.

comment:10 Changed 7 years ago by m.thorade@…

  • Resolution set to fixed
  • Status changed from new to closed

comment:11 Changed 7 years ago by adrpo

  • Milestone changed from Future to 1.12.0
Note: See TracTickets for help on using tickets.