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
Simulation of HeatExchangerSimulation fails #1758
Comments
Comment by dietmarw on 10 Aug 2015 15:17 UTC
This raises the question what "check" was actually done with Dymola 2016 (as stated in the release notes) and really needs to be fixed as quickly as possible. Are there any Test logs available from when the original test of build.3 is documented? |
Modified by dietmarw on 10 Aug 2015 15:22 UTC |
Comment by leo.gall on 10 Aug 2015 16:17 UTC The same error message exists in:
Regression tests focused on finding differences to MSL 3.2.1+build.2, these are documented in #1681 (based on MSL v3.2.1+build.3-rc1 Dymola 2014 FD01). Models have been simulated in Dymola 2014 FD01 with I do not know, if library officers or tool vendors ran checks / pedantic checks before release. |
Comment by dietmarw on 11 Aug 2015 06:34 UTC |
Comment by leo.gall on 11 Aug 2015 06:44 UTC
I agree, this should be the standard. For #799, probably Dymola 2014 was used. |
Comment by dietmarw on 11 Aug 2015 06:59 UTC |
Comment by anonymous on 11 Aug 2015 07:05 UTC
That is not exactly true.
|
Comment by leo.gall on 11 Aug 2015 07:23 UTC The goal of MA regression testing was to prevent unwanted changes in simulation results. Therefore, Dymola 2014 FD01 was used in order to not change to many variables at once (simulator version, library version). I totally agree, that there should be a final release check in the newest simulator versions (by tool vendors or library officers). If this was not done, the release notes would be wrong and we clearly have to improve the release process. |
Comment by hansolsson on 17 Aug 2015 08:15 UTC The e-mail sent after the testing was clear on that: (Without enabling Pedantic flag. Dymola 2016 FD01 should pass it also with Pedantic=true.) The only issues found are reported for ModelicaTest in https://trac.modelica.org/Modelica/ticket/1740 " From my perspective that was important - so that a new library could be downloaded and used in that version. The problem with cycles was detected, but not seen as a library problem at that time (see below). I don't recall the issue with start-values and will have to look at it again. However, the situation regarding cyclic bindings was worse that I thought: |
Comment by hansolsson on 17 Aug 2015 12:43 UTC
I have now looked once more at them. Note that they only fail if pedantic checks are enabled. The following are due to problems with cyclic parameter bindings which cannot be corrected with Modelica 3.2r2 semantics as far as I can see: Since this problem also occurred in previous builds of MSL 3.2.1 I cannot see that we can correct it in MSL 3.2.1 without introducing incompatibilities for users. The best we could do would be to document the issue, unless we want to create a 3.2r3 specification just to correct this. The following are start-value warnings that are non-blocking for the release of MSL: The reason they are non-blocking for the release of MSL is that none of them concern conflicting start-values for continuous state variables. There also seems to be other issues these diagnostics: the same error occurs more than once. Obviously we have worked on improving the diagnostics for future Dymola releases. |
Comment by fcasella on 24 Sep 2015 12:45 UTC The binding equations for parameters and constants in the translated model must be According to this, the cyclic dependencies should be looked for after flattening, and if that is done, there is in fact no dependency at all in the cited example models, as one of the two potentially involved binding equations is changed to a specific numerical value by a modifier when instantiating the components. So, there is really no issue about this in MSL 3.2.1. There might be problems when checking the class alone, as both default bindings are still presente, and this is covered by the improvements of MS 3.3r1 by evaluating the boolean parameters, but this is outside the scope of this ticket. As to the conflicting start values, first of all this concept has been introduced in Modelica 3.3, so it doesn't apply for MSL 3.2.1. Besides, the text of section 8.6.2 by no means implies that having conflicting start attributes on aliases with the same priority is by any means illegal - in fact the suggested procedure is non-normative. A warning might be issued, but this is definitely not an error. |
Comment by hansolsson on 24 Sep 2015 13:03 UTC
Flattening just resolves modifiers and names; it does not evaluate parameters as is needed here. That was why #1320 was needed. The reason for having the restriction after flattening is that someone can use:
(or likely using it in a better way).
I agree that there is no issue for the start-values. |
Comment by fcasella on 24 Sep 2015 13:26 UTC |
Comment by fcasella on 24 Sep 2015 21:06 UTC
As no modifiers are applied to these parameters when the model is instantiated in those examples, there is a potential circular dependency, if one just looks at the incidence matrix. However, if one looks carefully, conditions are mutually exclusive, so in fact there is never any circular dependency to be worried about. A tool could determine this easily by applying the rules defined in MS 3.3r1, i.e., evaluating use_eps_Re and then applying symbolic simplification, but we can't rely on a later version of the language for MSL 3.2.1. Anyway, by careful analysis we know there is actually no cyclic dependency here, so the model can never lead to implicit equations to determine parameters, and there will be no problem at all generating the code. The rule of MS 3.2r2 (The binding equations for parameters and constants in the translated model must be acyclic after flattening) indeed applies here, though tools usually do not employ sophisticated enough techniques to detect this. It is impossible to change the source code to eliminate the problem at the root, unless we introduce a backwards-incompatible change in the library. As many other libraries (including the Buildings library, which has a large user base) depend on this component, and the consequences of such a change might be significantly bad, we think we have a very strong argument in favour of not doing so. The conclusion after discussion at the 87th Design Meeting in Velizy is to keep this code in MSL 3.2.1 and add the following text to the release notes: The model Modelica.Fluid.Pipes.BaseClasses.FlowModels.DetailedPipeFlow contains two |
Modified by fcasella on 24 Sep 2015 21:07 UTC |
Comment by beutlich on 24 Sep 2015 21:55 UTC Should we Keep the ticket open as long as it is not in the release notes? |
Comment by fcasella on 24 Sep 2015 23:09 UTC |
Modified by fcasella on 24 Sep 2015 23:10 UTC |
Comment by otter on 28 Sep 2015 11:32 UTC Ticket #1758 states that simulation of Modelica.Fluid.Examples.HeatingSystem fails in Dymola 2016 if option "pedantic mode for checking Modelica semantics" is set. This issue was not fixed in the library due to the following reasons: The Modelica.Fluid library uses a particular pattern to define some parameters resulting in a cyclic dependency of parameters if only incident information is taken into account. According to Modelica Specification 3.2 revision 2 this is not allowed (and therefore Dymola 2016 correctly reports errors if the pedantic flag is set). In ticket #1320 this issue was resolved for Modelica Specification 3.3 revision 1 by allowing cyclic parameter definitions if the cycles disappear when evaluating parameters that have annotation Evaluate=true. Modelica.Fluid is correct with respect to Modelica Specification 3.3 revision 1. Changing the Modelica.Fluid library for build 4 so that no cyclic parameter dependencies would be present anymore would (a) result in a non-backwards compatible change and (b) make the usage of Modelica.Fluid less convenient. For this reason Modelica.Fluid is not changed. (Practically, this means for example that the pedantic flag in Dymola 2016 needs to be switched off, when using the Modelica.Fluid library in version 3.2.1 build 4 and any previous version). |
Modified by otter on 28 Sep 2015 11:54 UTC |
Changelog modified by otter on 28 Sep 2015 11:54 UTC |
Comment by fcasella on 28 Sep 2015 13:02 UTC
For the record, the Modelica Specification 3.2 revision 2 only requires that the translated model must be acyclic, but it doesn't say that this should be checked with incidence information only. So, IMHO the model is perfectly legal according to the specification, as it never leads to cyclical dependencies for any value of the parameters. It is just a tool issue if this is not detected properly. |
Reported by anonymous on 10 Aug 2015 11:04 UTC
Dymola 2016 fails on Modelica.Fluid.Examples.HeatExchanger.HeatExchangerSimulation of new MSL 3.2.1+build.3 with cyclic parameter bindings and conflicting start values.
Migrated-From: https://trac.modelica.org/Modelica/ticket/1758
The text was updated successfully, but these errors were encountered: