Skip to content
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

Wrong usage of quantities in Modelica.Electrical.Analog.Interfaces.IdealSemiconductor #4386

Open
AHaumer opened this issue Apr 16, 2024 · 3 comments
Labels
bug Critical/severe issue L: Electrical.Analog Issue addresses Modelica.Electrical.Analog
Milestone

Comments

@AHaumer
Copy link
Contributor

AHaumer commented Apr 16, 2024

I suppose the following two lines

  v = (s*unitCurrent)*(if off then 1 else Ron) + Vknee;
  i = (s*unitVoltage)*(if off then Goff else 1) + Goff*Vknee;

should be replaced by:

  v = s*(if off then unitVoltage else unitCurrent*Ron) + Vknee;
  i = s*(if off then unitVoltage*Goff else unitCurrent) + Goff*Vknee;

otherwise the quantities are horribly wrong?
It's the same for Modelica.Electrical.Analog.Interfaces.IdealSwitch.
Should we fix that for 4.1.0?
What's your opinion, @christiankral @casella @dietmarw @HansOlsson ?

@AHaumer AHaumer added bug Critical/severe issue L: Electrical.Analog Issue addresses Modelica.Electrical.Analog labels Apr 16, 2024
@HansOlsson
Copy link
Contributor

HansOlsson commented Apr 16, 2024

Currently I would say it is fine, and not an urgent bug - and it's not clear if it is a bug.

The s-parameter has unit "1" and we can uniquely determine that the number 1 has unit resistance in v = (s*unitCurrent)*(if off then 1 else Ron) + Vknee; One could make it clearer by writing v = (s*unitCurrent)*(if off then unitResistance else Ron) + Vknee;, but it's not clear if it is needed, and I'm more concerned with the unit-issues in the Spice-part.

@dietmarw
Copy link
Member

Definitely something for 4.1.1 or 4.2.0 since we are long past the code freeze for 4.1.0.

@dietmarw dietmarw added this to the MSL4.2.0 milestone Apr 16, 2024
AHaumer pushed a commit to AHaumer/ModelicaStandardLibrary that referenced this issue Apr 16, 2024
@AHaumer
Copy link
Contributor Author

AHaumer commented Apr 16, 2024

@HansOlsson
but it's weird to fiddle around with "unitCurrent" and "unitVoltage" and leave it up to the tool to determine the unit of "1".
I've already proposed an easy-to-read fix.
Spice ... sigh ... Who should take care about that? Who is using that? Can we get rid of that?

beutlich pushed a commit to AHaumer/ModelicaStandardLibrary that referenced this issue Aug 14, 2024
casella pushed a commit that referenced this issue Aug 16, 2024
…s.IdealSemiconductor (#4387)

* fixed bug #4386

* same issue with Electrical.Analog.Ideal.{IdealTwoWaySwitch, IdealIntermediateSwitch, ControlledIdealTwoWaySwitch, ControlledIdealIntermediateSwitch}

---------

Co-authored-by: Anton Haumer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Critical/severe issue L: Electrical.Analog Issue addresses Modelica.Electrical.Analog
Projects
None yet
Development

No branches or pull requests

3 participants