ModelicaStandardLibrary icon indicating copy to clipboard operation
ModelicaStandardLibrary copied to clipboard

Fix and add unit definitions in Spice3

Open modelica-trac-importer opened this issue 8 years ago • 12 comments

Modified by dietmarw on 28 Sep 2010 11:36 UTC The Spice3 library still has a number of unit-less variables representing physical quantities. Some quantities have wrong units defined. This should be improved by fixing the existing units and adding unit definitions to all physical quantities.

Original description:

,,When doing a check of Modelica.Electrical.Spice3 I'm getting about 217 Warnings about incompatible units. This is normally an indicator for ill-designed equations and parameters and should be avoided and fixed if possible.,,


Reported by dietmarw on 20 Sep 2010 08:58 UTC When doing a check of Modelica.Electrical.Spice3 I'm getting about 217 Warnings about incompatible units. This is normally an indicator for ill-designed equations and parameters and should be avoided and fixed if possible.


Migrated-From: https://trac.modelica.org/Modelica/ticket/414

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Modified by dietmarw on 20 Sep 2010 08:58 UTC

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by dietmarw on 20 Sep 2010 09:17 UTC When inspecting the code I also found that Spice3 is using a duplicate definition of the Boltzmann constant

Instead of defining

constant Real CONSTboltz(  final unit= "J/K") =  (1.3806226e-23);

Modelica.Constants.k should be used. This is defined as:

final constant Real k(final unit="J/K") = 1.3806505e-23 "Boltzmann constant";

Not quite sure why those two values differ though.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by dietmarw on 20 Sep 2010 09:25 UTC The following change in SpiceConstants will reduce the number of warnings to 81:

constant Real CONSTvt0(   final unit= "(J/K)/(A.s)") = 
-                                     CONSTboltz * (27 + CONSTCtoK)  / CHARGE; // deg C
+ Modelica.Constants.k * Modelica.SIunits.Conversions.from_degC(27)  / CHARGE; // deg C

And should be more error prone.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by dietmarw on 27 Sep 2010 05:28 UTC In f7b1f61de0557630f2c99877529a54e3910171e2 the unit warnings were "fixed" by removing the unit definitions. Now we no longer get warnings but this is simply because the tool no longer can check the units. I think the real reason for some unit checks failing is that some units might be defined incorrectly.

Take the example from above: CONSTvt0 is defined with unit= "(J/K)/(A.s)" but looking at the actual equation

constant Real CONSTvt0(   final unit= "(J/K)/(A.s)") =   
 Modelica.Constants.k * Modelica.SIunits.Conversions.from_degC(27)  / CHARGE; // deg C

we get unit wise:

[J/K] * [K] / [A.s] = J/(A.s) != (J/K)/(A.s)

Not being very familiar with semiconductors I might be missing something hee but I guess it's better to fix the unit warnings by fixing the equations rather then simply disabling the unit check :)

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by clauss on 27 Sep 2010 06:32 UTC I was asked to get rid of the warnings "quickly" for the upcoming release. Therefore I changed a few units to Real. I'm aware that this is not a good way, and I'm also aware, that many other more or less internal variables are of type Real instead of the correct unit, and, I cannot exclude it, internal units may be defined incorrectly. The true reason is that the original equations in SPICE3 are written without units, and we must add them which is not ready yet due to being not quit easy. Therefore, in spring this year we found the compromise to give at least parameters the correct unit. We plan to completely add all units until the end of the Modelisar project, possibly earlier, but not within the next week. I'd suggest to keep this ticket for the following release.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by dietmarw on 28 Sep 2010 11:36 UTC OK I've updated the title and the description and rescheduled the ticket for the next release.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by dietmarw on 2 Aug 2012 13:13 UTC I wonder what the status is here. A quick grep still showed more than 700 variables and parameters defined as Real. This seems rather a lot to me.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by clauss on 7 Aug 2012 13:32 UTC The status is the same as 23 months ago, unfortunately. Differently from planning there was no opportunity to complete the units in the Modelisar project. At present we definitely do not have enough manpower for this.

We plan do fix this ticket in future, but it is not possible until the next MSL release.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Modified by dietmarw on 7 Aug 2012 13:39 UTC

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Comment by fcasella on 29 May 2013 12:32 UTC My suggestion is that variables inside the equation sections and in function inputs and outputs should have the correct physical units, but that internal variables of function algorithms are kept as Reals.

The reason (see also the discussion in https://github.com/modelica/ModelicaStandardLibrary/issues/1134#issuecomment-272636267) is that these functions are a one-to-one conversion of procedural FORTRAN code, which was not originally designed with unit checking in mind, and might in some case actually be impossible to fully characterize with consistent units.

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Changelog removed by fcasella on 29 May 2013 12:32 UTC

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer

Modified by otter on 18 Jul 2013 12:06 UTC

modelica-trac-importer avatar Jan 14 '17 03:01 modelica-trac-importer