modelica / ModelicaStandardLibrary

Free (standard conforming) library to model mechanical (1D/3D), electrical (analog, digital, machines), magnetic, thermal, fluid, control systems and hierarchical state machines. Also numerical functions and functions for strings, files and streams are included.
https://doc.modelica.org
BSD 3-Clause "New" or "Revised" License
452 stars 165 forks source link

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

Open AHaumer opened 2 months ago

AHaumer commented 2 months ago

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 ?

HansOlsson commented 2 months ago

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 commented 2 months ago

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

AHaumer commented 2 months ago

@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?