Subtyping violations according to System Modeler

[henrikt-ma]

In the interest of contributing to the discussion in modelica/ModelicaSpecification#3578, I'd like to be able to try the examples of this library in Wolfram System Modeler. Unfortunately the examples I have tried are rejected due to subtype relations not being fulfilled. Is this an error in the library, or is System Modeler rejecting invalid models?

Starting with BenchmarkCammarata, the first reported violation looks like this:

Error: SOFCPoliMi.ParametrizedModels.StackCammarata 3:15-3:15 Incompatible classes. Components.FuelCell.ChemicalReactions.ChannelReactionRatesAir is not a subtype of Components.FuelCell.ChemicalReactions.ChannelReactionRatesCO2, the constraint was imposed here. The following elements are missing or have incompatible types themselves:
.SOFCPoliMi.Types.MolarEnergy EaWGS;
.SOFCPoliMi.Types.SpecificMolarGibbsFreeEnergy dg0SRC2H6;
.SOFCPoliMi.Types.SpecificMolarGibbsFreeEnergy dg0SRC3H8;
.SOFCPoliMi.Types.SpecificMolarGibbsFreeEnergy dg0SRCH4;
Real etaSRC2H6;
Real etaSRC3H8;
Real etaSRCH4;
Real etaWGS;
.SOFCPoliMi.Types.PerUnit kEqSRC2H6;
.SOFCPoliMi.Types.PerUnit kEqSRC3H8;
.SOFCPoliMi.Types.PerUnit kEqSRCH4;
.SOFCPoliMi.Types.PerUnit kPSRC2H6;
.SOFCPoliMi.Types.PerUnit kPSRC3H8;
.SOFCPoliMi.Types.PerUnit kPSRCH4;
.SOFCPoliMi.Types.PerUnit kPWGS;
.SOFCPoliMi.Components.FuelCell.ChemicalReactions.SRc2h6 srC2H6;
.SOFCPoliMi.Components.FuelCell.ChemicalReactions.SRc3h8 srC3H8;
.SOFCPoliMi.Components.FuelCell.ChemicalReactions.SRch4 srCH4;

The source location is pointing here:

model StackCammarata
  extends Components.FuelCell.Stack(
    redeclare model ReactionRates =

Would it be possible to solve this by introducing a more restrictive constrainedby in Stack, instead of getting the ChannelReactionRatesCO2 as implicit constraint?

On a different topic which doesn't seem big enough for opening a separate issue just yet, System Modeler complains about missing each in SOFCPoliMi.Components.FuelCell.Stack:

    redeclare model Fluid = Fluid,
    redeclare model ReactionRates = ReactionRates,
    redeclare model PENOhmRes = PENOhmRes,

[casella]

In the interest of contributing to the discussion in modelica/ModelicaSpecification#3578, I'd like to be able to try the examples of this library in Wolfram System Modeler. Unfortunately the examples I have tried are rejected due to subtype relations not being fulfilled. Is this an error in the library, or is System Modeler rejecting invalid models?

It's likely that the models are invalid, unfortunately neither OpenModelica nor Dymola are apparently strict enough to check subtyping constraints in simulation models.

I'll ask @perost to check that and open a ticket with DS asap. I'll add the constrainedby clauses to the source code ASAP.

Would it be possible to solve this by introducing a more restrictive constrainedby in Stack, instead of getting the ChannelReactionRatesCO2 as implicit constraint?

Absolutely.

On a different topic which doesn't seem big enough for opening a separate issue just yet, System Modeler complains about missing each in SOFCPoliMi.Components.FuelCell.Stack:

    redeclare model Fluid = Fluid,
    redeclare model ReactionRates = ReactionRates,
    redeclare model PENOhmRes = PENOhmRes,

I guess you are referring to these lines:

SOFCPoliMi/SOFCPoliMi/Components/FuelCell/Stack.mo

Lines 88 to 91 in 4269463

	SOFCPoliMi.Components.FuelCell.Module module[N](
	redeclare model Fluid = Fluid,
	redeclare model ReactionRates = ReactionRates,
	redeclare model PENOhmRes = PENOhmRes,

That's a good point. The reason we didn't put each there, is that I understand it is not actually possible to have different redeclares for each element of the module array, I don't think we even have a syntax to do that, so I guess each is completely redundant here. @perost, would like to comment on that?

[perost]

That's a good point. The reason we didn't put each there, is that I understand it is not actually possible to have different redeclares for each element of the module array, I don't think we even have a syntax to do that, so I guess each is completely redundant here. @perost, would like to comment on that?

As you say there's no way to redeclare individual elements, but that doesn't mean that redeclares are exempt from the each rule. But it seems like OpenModelica doesn't check for each in this particular case even though it should.

[casella]

As you say there's no way to redeclare individual elements, but that doesn't mean that redeclares are exempt from the each rule. But it seems like OpenModelica doesn't check for each in this particular case even though it should.

I guess that would be a good reason to change the rule (Ockham rules!), but as long as it is in force, I'll fix the code to follow it.

[henrikt-ma]

I guess that would be a good reason to change the rule (Ockham rules!)

Can you remind me of a single case where the each is not redundant? My memories are a bit vague, but I'm afraid the each semantics are not powerful enough to provide more than redundancy…

[casella]

I understood from @perost that having this indication is sparing him a lot of work in potentially ambiguous situations. Maybe he can comment on that directly.

[casella]

On a different topic which doesn't seem big enough for opening a separate issue just yet, System Modeler complains about missing each in SOFCPoliMi.Components.FuelCell.Stack:

Fixed in ab05ba4.

[casella]

Would it be possible to solve this by introducing a more restrictive constrainedby in Stack, instead of getting the ChannelReactionRatesCO2 as implicit constraint?

Should be fixed by 27cfa95. Please try again.

[perost]

I understood from @perost that having this indication is sparing him a lot of work in potentially ambiguous situations. Maybe he can comment on that directly.

I don't think I've ever claimed that it makes anything easier, only that there are situations where not having each would lead to ambiguity. I know I've showed you some example before, but I can't come up with one off the top of my head I'm afraid.

[henrikt-ma]

I needed to change two more places for BenchmarkCammarata:

diff --git a/SOFCPoliMi/Components/FuelCell/AnodeChannel.mo b/SOFCPoliMi/Components/FuelCell/AnodeChannel.mo
index 6cd2e2a..be433e3 100644
--- a/SOFCPoliMi/Components/FuelCell/AnodeChannel.mo
+++ b/SOFCPoliMi/Components/FuelCell/AnodeChannel.mo
@@ -3,7 +3,7 @@ model AnodeChannel
   extends Components.FuelCell.BaseChannel(isAnode = true, alphaNom = 510, Nu = 2.7);
   replaceable model HOR = ChemicalReactions.HOR;
   replaceable model ReactionRates =
-      Components.FuelCell.ChemicalReactions.ChannelReactionRatesCO2;
+      Components.FuelCell.ChemicalReactions.ChannelReactionRatesCO2 constrainedby Components.FuelCell.ChemicalReactions.BaseClasses.ChannelReactionRates;
   // Substances and Reaction Coefficients
   constant Real massStoichHOR[nX] = stoichHOR.*fluidIn.MM "";
   constant Real massStoichSR[nX] = stoichSR.*fluidIn.MM "";
diff --git a/SOFCPoliMi/Components/FuelCell/Module.mo b/SOFCPoliMi/Components/FuelCell/Module.mo
index 32c43dc..0e47baa 100644
--- a/SOFCPoliMi/Components/FuelCell/Module.mo
+++ b/SOFCPoliMi/Components/FuelCell/Module.mo
@@ -1,7 +1,7 @@
 within SOFCPoliMi.Components.FuelCell;
 model Module
   replaceable model ReactionRates =
-      Components.FuelCell.ChemicalReactions.ChannelReactionRatesCO2;
+      Components.FuelCell.ChemicalReactions.ChannelReactionRatesCO2 constrainedby Components.FuelCell.ChemicalReactions.BaseClasses.ChannelReactionRates;
   replaceable model PENOhmRes = Components.FuelCell.OhmicResistancePEN;
   replaceable model Fluid =
       Media.MainClasses.SOS_CO2.SOS10ComponentsModelica;
diff --git a/SOFCPoliMi/Media/BaseClasses/IdealModelicaMedia/package.order b/SOFCPoliMi/Media/BaseClasses/IdealModelicaMedia/package.order

I am now investigating the problems appearing next…

[casella]

I know I've showed you some example before, but I can't come up with one off the top of my head I'm afraid.

Neither can I. But I think it would be important to have them clear. Either 'each' is necessary or it is redundant. If it is necessary, we just need to always put it when needed, as the MLS says. If it isn't, then we could discuss whether to make it optional or even remove it.

[casella]

I needed to change two more places for BenchmarkCammarata:

Done in c08f438

I am now investigating the problems appearing next…

Good luck with that 😅

[henrikt-ma]

I am now investigating the problems appearing next…

Good luck with that 😅

So far I have failed to make sense of our error messages, so I suggest closing this issue as fixed.

For the purpose of contributing to modelica/ModelicaSpecification#3578, any chance we could try a Base Modelica variant of the model?

[casella]

I tried to export it but the example is definitely non-trivial and we have a glitch with constant start attributes in Medium records 😓

I'll try to provide one ASAP.

[casella]

See OpenModelica/OpenModelica#12958