Use of partial package SingleGasNasa

[modelica-trac-importer]

Reported by hansolsson on 27 Apr 2012 10:15 UTC
Classes and constants in partial packages should not be used in simulation models in Modelica (except as part of base-packages etc).

However, classes from the partial package Modelica.Media.IdealGases.Common.SingleGasNasa are used directly in three variants:

1. The ticket Bug in Modelica.Media.Examples.MixtureGases? #710 found one issue that could be avoided by not using the functions from that partial package in MixtureGasNasa. The functions used are at least: h_T, cp_T, s0_T, dynamicViscosityLowPressure, thermalConductivityEstimate

2. All constants in Modelica.Media.IdealGases.Common.FluidData
   use SingleGasNasa.FluidConstants.

3. MixtureGasNasa.BaseProperties has an import of SingleGasNasa (should probably just be removed).

The proposal for solving the function issue for #710 also applies for those constants, included below with better package name (still subject to change):

  partial package SingleGasNasa
    constant IdealGases.Common.DataRecord data;
    ...
    function thermalConductivityEstimate
      ...
    algorithm
      lambda:=SingleGasCommon.thermalConductivityEstimate(..., data)
    end thermalConductivityEstimate;
  end SingleGasNasa;

  partial package MixtureGasNasa
    ..
      SingleGasCommon.thermalConductivityEstimate(...)
    ..
  end MixtureGasNasa;

  package SingleGasCommon
     record FluidConstants end FluidConstants;
     function thermalConductivityEstimate
        input IdealGases.Common.DataRecord data;
        ...
      algorithm
        lambda:=...;
      end thermalConductivityEstimate;
  end SingleGasCommon;

  package FluidData
     constant SingleGasCommon.FluidData N2=...;
     ...
  end FluidData;

Have not yet found any other uses of partial packages in MSL.

---

Migrated-From: https://trac.modelica.org/Modelica/ticket/731

[modelica-trac-importer]

Comment by hubertus on 29 Apr 2012 21:40 UTC
This was fixed in the MediaFix branch in 3cbe4b8, but not merged to trunk or other maintenance branches. The issue with the FluidConstants record had been fixed there already earlier.

[modelica-trac-importer]

Comment by hansolsson on 2 May 2012 08:08 UTC
Good that it is worked on, although I am not certain that 3cbe4b8 is fully correct yet.

The h_Tlow_der has in green:
input SI.SpecificEnthalpy h_off=SingleGasNasa.h_offset
which to me should be:
input SI.SpecificEnthalpy h_off=h_offset

And some other code has:
input Interfaces.PartialMedium.DipoleMoment mu
which probably should be the old:
input DipoleMoment mu
(and otherwise just the Modelica.SIunits one).

Basically it is suspicious if there is any dot-notation with a partial package, e.g., the following strings in the code seems suspicious in general: PartialMedium.<..>, SingleGasNasa.<..>

[modelica-trac-importer]

Comment by hansolsson on 3 May 2012 12:03 UTC
I looked once more: the use of Interfaces.PartialMedium.... in the functions above cannot be correct.
In addition we have:

4. Import of classes from PartialMedium, e.g. SingleGasNasa has:
   import Modelica.Media.Interfaces.PartialMedium.Choices.ReferenceEnthalpy;

It might be that it is necessary to also move Choices out of PartialMedium.

[modelica-trac-importer]

Comment by hansolsson on 3 May 2012 14:42 UTC
5. An issue with Choices in PartialMedium is the ThermoStates constant in PartialMedium:

    constant Modelica.Media.Interfaces.PartialMedium.Choices.IndependentVariables
    ThermoStates "Enumeration type for independent variables";
...
    constant AbsolutePressure reference_p=101325 
    "Reference pressure of Medium: default 1 atmosphere";

Since PartialMedium is partial the use of full name for ThermoStates cannot be correct (this contstant is used in e.g. Fluid-models).
The correction is to either use a local name (similar to the reference_p):

    constant Choices.IndependentVariables
    ThermoStates "Enumeration type for independent variables";
...
    constant AbsolutePressure reference_p=101325 
    "Reference pressure of Medium: default 1 atmosphere";

or move Choices-package out of PartialMedium.

[modelica-trac-importer]

Modified by hansolsson on 22 May 2012 12:11 UTC

[modelica-trac-importer]

Modified by hubertus on 22 May 2012 12:15 UTC

[modelica-trac-importer]

Comment by hubertus on 28 May 2012 22:12 UTC
All issues reported aboe have been fixed in b6893f2. Not yet closing, needs further looking at the code.

[modelica-trac-importer]

Comment by hubertus on 10 Jun 2012 23:31 UTC
Latest version of Dymoal did not find any partial package issues any more in r 5180. Closing for now.

[modelica-trac-importer]

Comment by sjoelund.se on 22 Aug 2012 11:44 UTC
So the fix was to move Choices from PartialMedium to PartialSimpleIdealGasMedium?

But Water extends Interfaces.PartialTwoPhaseMedium which extends PartialPureSubstance which extends PartialMedium

And not PartialSimpleIdealGasMedium! So it seems to me this is what is breaking OpenModelica at the moment (Choices not found in Water tests). I reverted revisions one at a time until I found this. How does this work in Dymola? Or did I miss some extends somewhere? Was something changed in a later revision? I assumed not since HEAD also does not work, but there may be multiple issues.

[modelica-trac-importer]

Changelog removed by sjoelund.se on 22 Aug 2012 11:44 UTC

[modelica-trac-importer]

Comment by hubertus on 22 Aug 2012 11:53 UTC
Exactly what revision in which branch are you using? Choices are located in Modelica.Media.Interfaces in trunk, and it should be found by the normal lookup rules. I assume something else is the trouble with HEAD for you, I cannot follow your argument.

[modelica-trac-importer]

Comment by sjoelund.se on 22 Aug 2012 12:05 UTC
This was diff between 5146 and 5147 and since I cannot view these changes in a model editor, I read it through text editor. I seem to have gotten the change wrong by one scope. Sigh. Ok, it is in Modelica.Media.Interfaces now, and was in PartialMedium before.

But then it will not be found, right? Since Choices is no longer existing in PartialMedium, the modifier Choices.IndependentVariables.ph in packages Modelica.Media.Water.XXX cannot be found in any inherited package, and not by going up the scopes either.

Doesn't the package structure then look something like:

package Interfaces

constant Real choices = 1.5;

model M
  Real r;
end M;

end Interfaces;

model N
  extends P.M(r = choices /* Cannot be found */);
end N;

[modelica-trac-importer]

Comment by sjoelund.se on 22 Aug 2012 12:19 UTC
The following diff would resolve the issue that "extends WaterIF97_base(ThermoStates = Choices.IndependentVariables.ph" and similar in Water cannot find the correct choice. Another alternative would be to import Modelica.Media.Interfaces.Choices in all packages that modify this, but that is a non-backwards-compatible change, no?

===================================================================
--- Modelica/Media/package.mo	(revision 5418)
+++ Modelica/Media/package.mo	(working copy)
@@ -4026,6 +4026,7 @@
 
     import SI = Modelica.SIunits;
     extends Modelica.Icons.Library;
+    package Choices = Modelica.Media.Interfaces.Choices;
 
     // Constants to be set in Medium
     constant Modelica.Media.Interfaces.Choices.IndependentVariables

[modelica-trac-importer]

Comment by hubertus on 22 Aug 2012 12:19 UTC
Your example is a bit weird (where is P defined ?). The uses I can find and see should be found by normal lookup rules. All uses of Choices I could find use an absolute path starting at Modelica.Media. etc.

[modelica-trac-importer]

Comment by hubertus on 22 Aug 2012 12:23 UTC
I think we have some trouble with the branch that is used. The Media-Fix branch is old and I should probably delete it. All known issues are fixed in trunk, and I merge them also to 3.2.1. I do not plan to maintain other branches, but of course anyone is free to do so.

[modelica-trac-importer]

Comment by sjoelund.se on 22 Aug 2012 12:29 UTC
Yes, trunk is fine. I'll make all the references in MediaFix into the absolute path.