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

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

Closed
modelica-trac-importer opened this issue Jan 14, 2017 · 24 comments
Assignees
Labels
bug Critical/severe issue L: Electrical.Analog Issue addresses Modelica.Electrical.Analog
Milestone

Comments

@modelica-trac-importer
Copy link

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 modelica-trac-importer added bug Critical/severe issue L: Electrical.Analog Issue addresses Modelica.Electrical.Analog labels Jan 14, 2017
@modelica-trac-importer
Copy link
Author

Modified by dietmarw on 20 Aug 2013 16:43 UTC

@modelica-trac-importer modelica-trac-importer added this to the MSL3.2.2 milestone Jan 14, 2017
@modelica-trac-importer
Copy link
Author

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
Copy link
Author

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

@modelica-trac-importer
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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.

  • 1.60217646e-19
  • 1.6021892e-19
  • 1.6021918e-19
    Please address this issue for MSL 3.2.2 (expected to be released by end of 2015 according to the minutes of the last MDM in Velizy).

@modelica-trac-importer
Copy link
Author

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 9f680f6.

@modelica-trac-importer
Copy link
Author

Modified by majetta on 4 Nov 2015 10:41 UTC

@modelica-trac-importer
Copy link
Author

Modified by beutlich on 4 Nov 2015 10:46 UTC

@modelica-trac-importer
Copy link
Author

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

@modelica-trac-importer
Copy link
Author

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
Copy link
Author

Modified by beutlich on 4 Nov 2015 11:24 UTC

@modelica-trac-importer
Copy link
Author

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

@modelica-trac-importer
Copy link
Author

Comment by dietmarw on 4 Nov 2015 11:34 UTC
I looked a bit closer at the change that was done in 9f680f6 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
Copy link
Author

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

@modelica-trac-importer
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@modelica-trac-importer
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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
Copy link
Author

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

@modelica-trac-importer
Copy link
Author

Modified by ahaumer on 23 Dec 2015 19:53 UTC

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