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
470 stars 167 forks source link

Boltzmann and electron charge constants should not be redefined in Heating devices #1241

Closed modelica-trac-importer closed 7 years ago

modelica-trac-importer commented 7 years ago

Reported by jriel on 9 Aug 2013 22:29 UTC In the HeatingDiode component, the Boltzmann constant is declared as a protected variable:

Real k=1.380662e-23 "Boltzmann's constant, J/K";

In the HeatingNPN and HeatingPNP components, the Boltzmann constant is defined as an unprotected parameter:

parameter Real K=1.3806226e-23 "Boltzmann's constant";

Note that the values differ. All should be using constants, specifically Modelica.Constants.k.

The same issues occurs with the electron charge (q) in the same models. Alas, there currently is no Modelica.Constants.q0, though there should be.


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

modelica-trac-importer commented 7 years ago

Modified by dietmarw on 20 Aug 2013 16:43 UTC

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 4 Sep 2013 09:33 UTC Note: Reusing Modelica.Constant also make it easier to later update the physical constants to best known values, #1266 - and Modelica.Constants seem more up-to-date than these values.

And isn't the electron charge given as: q=F/N_A?

modelica-trac-importer commented 7 years ago

Comment by beutlich on 6 Oct 2015 09:31 UTC @Kristin: Do you think you can fix it?

modelica-trac-importer commented 7 years ago

Comment by Matthis Thorade on 29 Oct 2015 17:01 UTC I would also like to see elementary charge q0 added to Modelica.Constants. Yes, q0=F/N_A; is correct, for k one can also use k=R/N_A;

https://en.wikipedia.org/wiki/Boltzmann_constant

https://en.wikipedia.org/wiki/Elementary_charge#In_terms_of_the_Avogadro_constant_and_Faraday_constant

modelica-trac-importer commented 7 years ago

Comment by hansolsson on 30 Oct 2015 15:52 UTC Replying to [comment:4 Matthis Thorade m.thorade@…]:

I would also like to see elementary charge q0 added to Modelica.Constants. Yes, q0=F/N_A; is correct, for k one can also use k=R/N_A;

https://en.wikipedia.org/wiki/Boltzmann_constant

https://en.wikipedia.org/wiki/Elementary_charge#In_terms_of_the_Avogadro_constant_and_Faraday_constant

And in a few years the following might be exact (for some values of X), which can be seen as a strong argument for exposing them (at least after that change)- instead of computed values such as F and R (obviously we shouldn't remove F, but can keep it as F=q0*N_A;).

q0=1.60217Xe-19; k=1.3806Xe–23; N_A=6.02214Xe23; h=6.62606Xe–34

http://www.bipm.org/en/measurement-units/new-si/

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 31 Oct 2015 22:53 UTC Replying to [comment:5 hansolsson]:

And in a few years the following might be exact (for some values of X), which can be seen as a strong argument for exposing them (at least after that change)- instead of computed values such as F and R (obviously we shouldn't remove F, but can keep it as F=q0*N_A;).

q0=1.60217Xe-19; k=1.3806Xe–23; N_A=6.02214Xe23; h=6.62606Xe–34

http://www.bipm.org/en/measurement-units/new-si/

Hans, it would probably be a good idea to file a separate ticket for this.

modelica-trac-importer commented 7 years ago

Comment by beutlich on 4 Nov 2015 07:40 UTC Replying to [comment:3 beutlich]:

@Kristin: Do you think you can fix it?

There are now three different values of q used inside the SemiConductors package which is far from good modelling.

modelica-trac-importer commented 7 years ago

Comment by majetta on 4 Nov 2015 10:41 UTC I added the elementary charge q0 to the Modelica.Constants package and used it and Modelica.Constants.k in the models Modelica.Electrical.Analog.Semiconductors.Diode2, Modelica.Electrical.Analog.Semiconductors.HeatingNPN, Modelica.Electrical.Analog.Semiconductors.HeatingDiode, Modelica.Electrical.Analog.Semiconductors.HeatingPNP to replace k and q that were internally declared in that models. This was done in ticket 9f680f654446dc2220eb3ea8eecf0c632268163c.

modelica-trac-importer commented 7 years ago

Modified by majetta on 4 Nov 2015 10:41 UTC

modelica-trac-importer commented 7 years ago

Modified by beutlich on 4 Nov 2015 10:46 UTC

modelica-trac-importer commented 7 years ago

Changelog modified by beutlich on 4 Nov 2015 10:46 UTC Added atomic unit of charge as Modelica.Constants.q0

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 4 Nov 2015 11:22 UTC Please remove q0 again from Modelica.Constants. Adding things here should at least be coordinated with the library officer, i.e., Martin Otter.

I'd suggest you rely on the existing constants and use them like proposed above:

q0=Modelica.Constants.F/Modelica.Constants.N_A;

in your library. Then if in future it was found agreement on adding the elementary charge directly you can still update your usage of it.

modelica-trac-importer commented 7 years ago

Modified by beutlich on 4 Nov 2015 11:24 UTC

modelica-trac-importer commented 7 years ago

Changelog removed by beutlich on 4 Nov 2015 11:24 UTC

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 4 Nov 2015 11:34 UTC I looked a bit closer at the change that was done in 9f680f654446dc2220eb3ea8eecf0c632268163c and you really should not redefine a constant as a parameter or even as variable and neither redefine a constant using another existing constant just to change the name of it. The proper way is the import statement here.

Example:

- Real k=Modelica.Constants.k "Boltzmann's constant, J/K";
+ import Modelica.Constants.k "Boltzmann's constant, [J/K]";

and

- parameter Real K=Modelica.Constants.k "Boltzmann's constant";
+ import K=Modelica.Constants.k "Boltzmann's constant, [J/K]";

BTW very unfortunate that the Boltzmann's constant is used as k and K in the same library. You might wanna file a ticket on this.

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 4 Nov 2015 12:45 UTC I've fixed those issues in a13594a99117b471355d8ac4fed6d0d926d76dec : 0df5fb8ed874e3ccaaec18cc44f210d063d73186

modelica-trac-importer commented 7 years ago

Comment by majetta on 4 Nov 2015 13:08 UTC Thanks for the fixing. But I am rather sure that those changes are not backwards compatible at least in the models HeatingNPN and HeatingPNP since k an q had been parameter before and now they are constants. Of course it is very unlikely that a user of the models changed k and q but when you look at it in a formal way I think it is not backwards compatible.

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 4 Nov 2015 16:04 UTC I agree about the non-backward compatibility but since those parameters should never been parameters to begin with and users that have set them to something else would have broken their models in the first place. So with the removal of those two parameters a user gets a warning/error if that parameter was changed and I think this is actually an improvement and worth "breaking" backward compatibility.

modelica-trac-importer commented 7 years ago

Comment by otter on 13 Dec 2015 18:31 UTC Since the ticket seems to be fixed, I closed it.

modelica-trac-importer commented 7 years ago

Changelog modified by otter on 13 Dec 2015 18:31 UTC Semiconductors package: Boltzmann and electron charge constants used or computed from Modelica.Constants constants

modelica-trac-importer commented 7 years ago

Comment by beutlich on 13 Dec 2015 18:40 UTC Yes, it is fixed, but the non-backward compatible change should be documented in the release notes (similar as done in #1757 for MSL 3.2.1+build.4).

modelica-trac-importer commented 7 years ago

Comment by dietmarw on 14 Dec 2015 07:29 UTC Re-opening so that it's not forgotten in the release notes. Once they are updated this can be closed.

modelica-trac-importer commented 7 years ago

Comment by ahaumer on 23 Dec 2015 19:52 UTC added to the release notes in 8d12a170f3b1121dcd509e83044db3bc39b9254e, therefore closing the ticket.

modelica-trac-importer commented 7 years ago

Modified by ahaumer on 23 Dec 2015 19:53 UTC