Deltares / rtc-tools

The Deltares toolbox for control and optimization of environmental systems.
GNU Lesser General Public License v3.0
0 stars 2 forks source link

WIP: Simulation PIMixin: get canonical name before saving data to the output object #1583

Closed SGeeversAtVortech closed 1 month ago

SGeeversAtVortech commented 1 month ago

In GitLab by @OJMvD on Oct 23, 2018, 14:20

Previously, the simulation PIMixin tried to save simulation results using the timeseries id as defined in rtcDataConfig.xml. For the variables for which the timeseries id did not match the canonical name resulting from the alias relation, this would lead to empty series in the timeseries_export.xml. This is solved by saving (and acquiring) model results using the canonical name.

SGeeversAtVortech commented 1 month ago

In GitLab by @codecov on Oct 23, 2018, 14:22

Codecov Report

Merging #197 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted Files Coverage Δ
src/rtctools/simulation/pi_mixin.py 69.93% <100%> (+0.37%) :arrow_up:
SGeeversAtVortech commented 1 month ago

In GitLab by @OJMvD on Oct 23, 2018, 14:23

@jvande42b, can you take a look at this issue and fix? I can supply you with an example case.

I'm not sure why this problem occurs. In the simulation PIMixin self.__output is an AliasDict, and I expected that the trickery I introduced in this branch would not be necessary. Does this mean that there is a deeper underlying issue (in which another fix is needed than I proposed), or was there indeed something wrong with the PIMixin itself?

I'm not too familiar with the workings of the alias dicts etc., that's why I ask you :)

SGeeversAtVortech commented 1 month ago

In GitLab by @OJMvD on Oct 23, 2018, 14:30

Example case here: BOSBrabant_v5_Olav_parameterisatie.zip

This is a model in its early stages, so don't worry about other strange things you see. Authored by Herman.

SGeeversAtVortech commented 1 month ago

In GitLab by @jvande42b on Oct 25, 2018, 05:14

Yes, you are right that self.__output is an AliasDict and internally it is using the same self.alias_relation. There ought to be no need to to use the alias relation again... not sure what is wrong.

SGeeversAtVortech commented 1 month ago

In GitLab by @OJMvD on Oct 26, 2018, 12:18

This may be due to an too old Pymoca version. I'll get back to you.

SGeeversAtVortech commented 1 month ago

In GitLab by @vreeken on Dec 16, 2018, 20:06

@OJMvD any update? Or should I close this until the issue presents itself again.

SGeeversAtVortech commented 1 month ago

In GitLab by @OJMvD on Dec 17, 2018, 06:59

Closing. Issue did not present itself again. Was most likely due to a Pymoca version mismatch.

SGeeversAtVortech commented 1 month ago

In GitLab by @OJMvD on Dec 17, 2018, 06:59

closed