Cannot pass a parameter value coming from a class to another class nested in another class!!

[ali.shirazi@…]

Hello,
I have been recently reporting a number of issues in relation to our OM library (i.e. SolarTherm available from: ​https://github.com/SolarTherm/SolarTherm/tree/alberto-merge) with the recent version of OMC, in particular the stable and the nightly build versions.

In this specific issue, which I should mention that never happened in the previous versions of OMC (e.g. V1.11), one of our example model (i.e. TroughWagner.mo available form: ​https://github.com/SolarTherm/SolarTherm/blob/alberto-merge/examples/TroughWagner.mo) breaks because OM fails to pass a value from the class instance “wea” (i.e. wea.lat which is a parameter showing the latitude of a location, which itself is read from a weather data file) to another class (i.e. OptEff) which is itself nested in another class instance called CL. It should be mentioned that the model TroughWagner.mo itself is an extension of a base class called “GenericSystem.mo” which is available from: ​https://github.com/SolarTherm/SolarTherm/blob/alberto-merge/SolarTherm/Systems/GenericSystem.mo.

Here is the piece of code in GenericSystem.mo that shows the issue that I have been trying to demonstrate:

SolarTherm.Models.Sources.Weather.WeatherSource wea(file=wea_file, delay=
wdelay);

SolarTherm.Models.CSP.CRS.HeliostatsField.SteeredCL CL(
redeclare model OptEff = SolarTherm.Models.CSP.CRS.HeliostatsField.FileOE (
file=opt_file, orient_north=if wea.lat < 0 then true else false),
A=if match_sam then 1.0273*A_field else A_field,
steer_rate=0.002,
target_error=0.0001,
actual_0=0.0); // ~8 minutes till fully on sun
// if large can be large source of missing energy

And below is the error I get when I run TroughWagner.mo in OMEdit:

[7] 11:18:24 Translation Error
[SolarTherm.Systems.GenericSystem: 120:19-120:67]: Variable wea.lat not found in scope SolarTherm.Models.CSP.CRS.HeliostatsField.SteeredCL$CL.

Finally, I should mention that the package SolarTherm (from “alberto-merge” branch: ​https://github.com/SolarTherm/SolarTherm/tree/alberto-merge/SolarTherm) must be imported to OMEdit before running the above model as it contains a number of dependencies required by the model.

I would appreciate it if you have a look and let me know what causes this error.

Thanks,
Ali Shirazi

[casella]

@ali.shirazi, as I discussed with your colleague at the Mathmod Conference in Vienna, if you want to ensure the best support in OMC and your library is on GitHub, the best option is to include it in our extended testsuite.

This way, we have the library in our pipeline, and you get ​automated reports on coverage and ​daily reports about regressions and improvements, which can help catching the kind of problem you report hear early on.

We just need to agree on the branch which is supposed to be used for the tests. We normally test the master branch, but of course each project has its own management rules. Should we use ​https://github.com/SolarTherm/SolarTherm/tree/alberto-merge instead of the master branch?

Referring to your specific problem, it is hopefully going to be solved by the new front end, see #4138 and roadmap. Once the library is in the testsuite you can check the status with that ​here and the regressions and improvements ​here.

It would also help if you open one SolarTherm specific ticket that contains references to all the individual tickets you opened so far, see #2894 for an example. This would help us track your issues better.

[john.pye@…]

Hi Francesco -- thanks for your comments there. We would be happy for you to incorporate testing of SolarTherm into your OM test suite. What I guess you are referring to here is whether or not OMC will swallow our models without errors, and perhaps whether they run to completion or not.

Our code has a test suite that attempts to run various models but in addition, where possible, compares the simulation results with expected outcomes. We also have various wrapper scripts that are being tested at the same time -- preprocessing and postprocessing of various kinds. ​This page contains details of how we run our tests.

Regarding the branch we want to test, our current master branch is a bit dated and we are trying to merge some new work into it, but we are hesitant about doing that until we can resolve the current OM bugs that we are seeing. So for now I suggest testing against our alberto-merge branch, and once we resolve the bugs, we will merge to master and from then on you could test against master. Does that sound OK?

Regarding the resolution to this particular issue -- the new front-end sounds like it could be a little way off. Do we need to be looking at workarounds in the shorter term?

Regarding your test suite links, I guess you have a particular process for Verification. Can you describe that?

[casella]

Hi John, nice to hear you! Comments inline

We would be happy for you to incorporate testing of SolarTherm into your OM test suite.

Good.

What I guess you are referring to here is whether or not OMC will swallow our models without errors, and perhaps whether they run to completion or not.

Yes. Additionally, we can compare the outputs to reference result files that you can provide us (see below).

Our code has a test suite that attempts to run various models but in addition, where possible, compares the simulation results with expected outcomes. We also have various wrapper scripts that are being tested at the same time -- preprocessing and postprocessing of various kinds. ​This page contains details of how we run our tests.

Regarding the branch we want to test, our current master branch is a bit dated and we are trying to merge some new work into it, but we are hesitant about doing that until we can resolve the current OM bugs that we are seeing. So for now I suggest testing against our alberto-merge branch, and once we resolve the bugs, we will merge to master and from then on you could test against master. Does that sound OK?

OK

Regarding the resolution to this particular issue -- the new front-end sounds like it could be a little way off.

That's a nice understatement :)

Seriously, if you are not seeing much progress on the new front end (see, e.g., ​here), the reason is that there is a (hoperfully) small set of known issues to be resolved that affect a large number of test cases.

We expect the number of successful test to jump to close to 100% once they are resolved, and also that most issues that the previous front end had will automatically be fixed, because of the better structure of the new one. Of course there could be other problems which are currently masked and get visible later, but we don't expect them to be many. On the other hand, the new front end is more rationally organized, so it should support cases like the ones that you report in a straightforward way, whereas fixing the current front end to support them would need awful workarounds, or even not be possible.

The new front-end will also provide much faster performance of OMEdit and of model generation in general, which is another good reason to work on it.

Do we need to be looking at workarounds in the shorter term?

We are really focusing all our efforts there, and in general it makes little sense to waste time on possibly complicated workarounds on the old one, which is going to be retired soon. Of course if you have particular needs of supporting one model now, and the fix is not difficult, we could consider that, but in general it makes more sense to focus on the new front-end. From this point of view, having your library in the testsuite will help a lot.

Regarding your test suite links, I guess you have a particular process for Verification. Can you describe that?

There are no scripts. All models with the experiment(StopTime) annotation are simulated, and their results compared to reference result files which are provided by the library developer, with some tolerances. It's the library developer's responsibility to evaluate reported verification failures, and in case they decide that the mismatch is acceptable or justified, to update the reference results.

So, if you generate some input data on the fly using scripts before the simulation, you may have to provide them as datasets in the Resources directory, and load them from there in your model. I guess all post-processing is out of scope here, as long as regression testing is the primary aim. We can discuss this further if necessary.

[casella]

@sjoelund.se, can you please add the alberto-merge branch of the library to the testuite?

Thanks!

[sjoelund.se]

Replying to casella:

@sjoelund.se, can you please add the alberto-merge branch of the library to the testuite?

That wouldn't do anything; the library has no experiment annotations.

[sjoelund.se]

Never mind; the branch has 1 experiment. I can try to add it.

[Ali.Shirazi@…]

Hi @sjoelund.se,
I just addded the experiment annotation to our all examples and test cases in "examples" and "tests" folders in SolarTherm alberto-merge branch.

I would appreciate it if you can please also add the these to the OM testuite.

Thanks,
ALi Shirazi

[sjoelund.se]

Ah, only examples with experiment annotation in a Modelica library will actually run with the testsuite. So SolarTherm.Examples, SolarTherm.Tests, etc. If you want them stored separate and run them in the testing, you would need a different package (say SolarThermExamples), and both SolarTherm and SolarThermExamples would need a version="alberto-merge" and SolarThermExamples would need a uses-annotation on the SolarTherm library (version="alberto-merge").

The examples are rather small so I would suggest that they would be part of the SolarTherm library itself although this might be slightly problematic for your unit testing (easiest solution to do this would be to move the examples into the library and have the current files extend the corresponding class in the library; I could probably provide a quick pull request for this if you want me to).

Also note that these examples are currently stored outside of the SolarTherm folder so they won't actually be seen in the installed version of the library (and also not CMake projects, etc).

[ali.shirazi@…]

Hello,

We made the "examples" directory a standalone Modelica library outside the SolarTherm library, and also added the experiment annotation with start and stop time to the example models that we would like to be tested in there.

The other thing that I do need to mention is that we have merged "alberto-merge" branch with out mater branch. As such, we would like you now to include our master branch to the OM test-suite. For the time being, we want only the "examples" library to be tested out on your testing server.

Please let me know how it goes.

Thanks,
Ali

[casella]

Hi folks,

I see from the ​Hudson reports that currently we have both a SolarTherm and a SolarTherm-alberto-merge instances.

I guess the first one refers to the latest commit on the master branch (@sjoelund.se, please confirm), while I guess based on comment:9 that the alberto-merge library is obsolete and could be removed from the testsuite, @ali.shirazi please confirm.

Last, but not least, it is not clear to me where is the standalone example library located. @ali.shirazi, please give the URL of the repository so that @sjoelund.se can add it to the testsuite. Also please make sure you have included the uses annotation on the example library to load the SolarTherm library, otherwise all the tests will be broken.

Thanks!

[ali.shirazi@…]

Hi,
The following is the URLs to SolarTherm and "examples" library, which are both in the master branch of our repo. I also confirm that I have included the "uses" annotation in the examples library in order to load SolarTherm.

SolarTherm:
​https://github.com/SolarTherm/SolarTherm/tree/master/SolarTherm

examples library:
​https://github.com/SolarTherm/SolarTherm/tree/master/examples

Thanks.

[casella]

Rescheduled to 1.14.0 after 1.13.0 releasee

[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