Closed henrikt-ma closed 3 weeks ago
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
inStack
, instead of getting theChannelReactionRatesCO2
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
inSOFCPoliMi.Components.FuelCell.Stack
:redeclare model Fluid = Fluid, redeclare model ReactionRates = ReactionRates, redeclare model PENOhmRes = PENOhmRes,
I guess you are referring to these lines: https://github.com/looms-polimi/SOFCPoliMi/blob/4269463ea9069cd2d706715af17b3616cb6517da/SOFCPoliMi/Components/FuelCell/Stack.mo#L88-L91
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?
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 guesseach
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.
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 foreach
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.
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…
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.
On a different topic which doesn't seem big enough for opening a separate issue just yet, System Modeler complains about missing
each
inSOFCPoliMi.Components.FuelCell.Stack
:
Fixed in ab05ba4.
Would it be possible to solve this by introducing a more restrictive
constrainedby
inStack
, instead of getting theChannelReactionRatesCO2
as implicit constraint?
Should be fixed by 27cfa955. Please try again.
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.
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…
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.
I needed to change two more places for
BenchmarkCammarata
:
Done in c08f438
I am now investigating the problems appearing next…
Good luck with that 😅
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 https://github.com/modelica/ModelicaSpecification/issues/3578, any chance we could try a Base Modelica variant of the model?
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.
See OpenModelica/OpenModelica#12958
@henrikt-ma we fixed the previous problem but we are still generating somewhat invalid Base Modelica code due to some record definition issue, see OpenModelica/OpenModelica#13009. As soon as that issue is resolved, I hope to be able to provide you with a fully functional Base Modelica version of the SOFC model.
In the interest of contributing to the discussion in https://github.com/modelica/ModelicaSpecification/issues/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:The source location is pointing here:
Would it be possible to solve this by introducing a more restrictive
constrainedby
inStack
, instead of getting theChannelReactionRatesCO2
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
inSOFCPoliMi.Components.FuelCell.Stack
: