Fixes #576 by replacing internal models by one utility model #2533
Conversation
Instead of having the no-accessible models `rn` and `rp` inside the example I've created **one** universally usable switched capacitor model instead. Note: When replacing `rn` and `rp` in `Modelica.Electrical.Analog.Examples.CauerLowPassSC` I noticed that this and the other examples are in dire need of an clean up since the coding style is sub fortunate (lots of unnecessary protected connectors with no real purpose).
dietmarw
added
the
L: Electrical.Analog
| parameter Modelica.SIunits.Time clock=1 "Clock"; | ||
| parameter Modelica.SIunits.Resistance R=1 "Resistance"; | ||
| Modelica.Blocks.Sources.BooleanPulse BooleanPulse(period=clock) annotation (Placement(transformation(extent={{-8,70},{12,90}}))); | ||
| Modelica.Electrical.Analog.Basic.Capacitor Capacitor(C=clock/max(Modelica.Constants.eps,abs(R))) |
There was a problem hiding this comment.
Does this pass the unit check?
There was a problem hiding this comment.
Well the unit is correctly derived both in Dymola and in OpenModelica though it really depends on how the max function works.
@HansOlsson, @sjoelund is that somehow specified?
I can always make it 100% sure by changing it to:
Modelica.Electrical.Analog.Basic.Capacitor Capacitor(C=clock/max(minR,abs(R)))
protected
constant Modelica.SIunits.Resistance minR = Modelica.Constants.eps "Helping constant for division by zero protection";but I think it might be overdoing it especially when it is not necessary.
There was a problem hiding this comment.
I've decided to implement it using a "oneOhm" helping constant. So this should now be bullet proof. I'm still interested in the general question if this would be necessary really.
There was a problem hiding this comment.
The exact unit-handling involving such cases is not yet fully standardized, and I will have to go through the details of the proposed MCP - but such cases should be handled.
Another solution is to just skip the guard - and just accept that bad parameter values lead to division by zero, the old models did not handle small resistances either (the difference was that it was specified using min-value - but here we want R to avoid a region around zero - which cannot be represented with min/max attribute).
There was a problem hiding this comment.
I like to keep the guard as I see this as an improvement so that should people end up using the new component don't end up with some error message just because of a 0 resistance. I hope that is acceptable for you.
|
I agree that the default values should be given as start and also that the example should then have all instances parameterised in order to not trigger the warning. I hope this is now good to go. |
beutlich
added
enhancement
Instead of having the no-accessible models
rnandrpinside the example I've created one universally usable switched capacitor model instead.Note: When replacing
rnandrpinModelica.Electrical.Analog.Examples.CauerLowPassSCI noticed that this and the other examples are in dire need of an clean up since the coding style is sub fortunate (lots of unnecessary protected connectors with no real purpose).Close #576.