modelica
Add Spice3-specific infiniteTime constant
This description still does not make sense as it doesn't explain which overflow.
Overflow is what you get when you add something to the "maximum representable finite floating-point number"
https://github.com/modelica/ModelicaStandardLibrary/blob/8e7876b5414475abab003c466ca49050812aeb01/ModelicaServices/package.mo#L184-L185
@HansOlsson if you know how to express that synthetically in a comment, that shouldn't be more than one or two lines, please go ahead.
It is an OpenModelica issue
No, it is a MSL issue. Modelica.Constants defines inf as the "Maximum representable finite floating-point number", and the Spice3 models add something to it, which doesn't make sense.
and we cannot know which other libraries use similar time values.
Why should other libraries be relevant? This PR defines a constant to be used exclusively for the Spice3 models.
I think it's clear that this model has an issue with Undefined Behaviour according to MLS 3.3, caused by an overflow in
Twidthfrom adding something to the largest representable number.
I (and as I understand @henrikt-ma ) don't think that is clear at all, and have tried to clarify that.
Additionally, the risk with introducing such hacks in MSL due to issues in specific tools, is that they might spread to other domains - and suddenly we have a model where a large stop-time would actually makes sense and later see that the model starts misbehaving. That risk can be reduced by using a substantially larger value - similarly as the previous value for Modelica.Constants.inf was 1e60.
And if we say that tools may scale values as they see fit any value may potentially overflow.
Sorry @casella I won't review that. I was never involved in the development of Spice3 and I have never worked with Spice3. Christoph Clauss and Kristin Majetta are no longer active. I doubt that Spice3 is used a lot out there in the world. I would prefer to discontinue this sub-library.
I think it's clear that this model has an issue with Undefined Behaviour according to MLS 3.3, caused by an overflow in
Twidthfrom adding something to the largest representable number.I (and as I understand @henrikt-ma ) don't think that is clear at all, and have tried to clarify that.
I think there are even more compelling arguments, see my comment in #4479, and also in MLS #3591. To make a long story short, assuming that expressions are considered literally in Modelica models is a highly questionable assumption; therefore, any reasoning on overflow strictly based on that assumption is also questionable.
IMHO that settles the matter about the fact that using the updated Modelica.Constants.inf in this model is a bad idea, from a conceptual point of view.
Additionally, the risk with introducing such hacks in MSL due to issues in specific tools, is that they might spread to other domains - and suddenly we have a model where a large stop-time would actually makes sense and later see that the model starts misbehaving.
For the record, I question the definition of "hack" for this PR. There is nothing hackish in there, from a modelling point of view it makes perfect sense. And I (as well as others) think that there are also valid conceptual reasons why the use of the current Modelica.Constants.inf is really not a good idea. So, it's not a hack, it's a bug fix.
I also think that the risk you fear of that large stop time to make sense is actually zero, given that the currently selected value for infiniteTime is 250 times the estimated life of the universe. Let's get real. Or should I say Real? 😉
That risk can be reduced by using a substantially larger value - similarly as the previous value for
Modelica.Constants.infwas1e60.
@HansOlsson despite what I just wrote I have no problems changing the limit from 1e20 s to 1e60 s, if that is enough to get this PR through and close this argument. Please let me know.
And if we say that tools may scale values as they see fit any value may potentially overflow.
No, I beg your pardon but this is a specious argument.
Of course tools won't scale values in an arbitrary way, they will do that to avoid very large or very small values. Looking at the SI values of physical quantities (not just time!) in any system I can think of, excluding cosmological stuff, it is very hard to get values outside the 1e-15 - 1e15 range. And it is really impossible to get values outside the 1e-40 -1e40 range. That is nothing compared to the dynamic range of double-precision numbers, which is 1e-308 to 1e308. So there are huge margins to avoid overflows. We just need to steer clear from the boundary, as others have acknowledged, and to do that with a healthy margin, given the huge headroom allowed. Even if you are worried that infinite^2 and infinite^3 should still be representable, that would mean using 1e100.
In fact, I still think 1e60 (the old value for Modelica.Constants.inf) was a very good one-fits-all compromise to practically (but very safely) represent infinity, as we don't have Inf (in the sense of IEEE 754) in Modelica. I don't know who set that specific number, but to me it was wise and made a lot of sense. It is 1e40 times larger the largest practical value of any reasonable physical variable in SI units, yet still leaving room for a scaling factor up to 1e15 (giving 1e75), followed by raising it up to the fourth power (that gives 1e300) while still leaving a healthy 1e8 (that's 100 million) margin before the gates of doom. I mean, who could ask for more?
So, I am still convinced that we should have a "large but safe value" defined in the MSL, at the old value of 1e60. But that's another story and it's out of scope for this PR, which is specifically about setting a reasonable "infinite" time threshold for signal generators. Let's get this done first, I really can't see any problem with it.
Sorry @casella I won't review that. I was never involved in the development of Spice3 and I have never worked with Spice3.
Me neither. The issue we are discussing here has nothing to do with electronic circuit modelling, it only involves a flexible pulse signal generator, that can degenerate to a ramp generator with some very large default values for the period and pulse width. Maybe you can still have a look a it.
Christoph Clauss and Kristin Majetta are no longer active. I doubt that Spice3 is used a lot out there in the world. I would prefer to discontinue this sub-library.
Could be a good idea, but not for MSL 4.1.0. In any case I would try to ask their opinion, if possible.
I would be in favor of defining generally two constants "near_small" and "near_inf" with magnitudes with a safety margin to Constants.small and Constants.inf, but small or large enough that they won't be used normally in models AND with values that indicate that these are not meaningful values, but numerically safe, e.g. near_small = 9.999e-99 near_inf = 9.999e+99
I would be in favor of defining generally two constants "near_small" and "near_inf" with magnitudes with a safety margin to Constants.small and Constants.inf, but small or large enough that they won't be used normally in models AND with values that indicate that these are not meaningful values, but numerically safe, e.g. near_small = 9.999e-99 near_inf = 9.999e+99
I can sort of understand that, but:
- Just use 1e100 and 1e-100
- I'm not sure where the small value would be used in practice
- A safe large value depends on how it is used, the only actual numerical issue we've had with large values was some fluid model that cubed(?) a value that could be Modelica.Constants.inf (causing overflow). That would work with this value, but it is better to rewrite the model (as was done) and there might be another model using a higher power or something else. And on the other hand I recalled some bad code in a basic numerical routine that failed already for 1e4 a few years back (we worked around it in Dymola). The latter case gives an indication of what will be a real problem with such constants: as soon as we have them they are likely to remain and possibly even spread even long after the tool- or model-issues are corrected. Thus they will decrease understandability and traceability. Or basically it seems more like numerical superstition than good modeling.
@HansOlsson it's to point a human reader to the fact "that's a special value, not just an unusual choice" 1e100 I would consider as larger but maybe meaningful value. 9.999e99 looks that weird that indicates "I'm a special value". The small value could be used to indicate infinitely short rise time.
@HansOlsson it's to point a human reader to the fact "that's a special value, not just an unusual choice" 1e100 I would consider as larger but maybe meaningful value. 9.999e99 looks that weird that indicates "I'm a special value". The small value could be used to indicate infinitely short rise time.
For me it just triggers the "price tag reflex", and i suspect someone might be trying to deceive me. 😜
@HansOlsson it's to point a human reader to the fact "that's a special value, not just an unusual choice" 1e100 I would consider as larger but maybe meaningful value. 9.999e99 looks that weird that indicates "I'm a special value". The small value could be used to indicate infinitely short rise time.
For me it just triggers the "price tag reflex", and i suspect someone might be trying to deceive me. 😜
Especially with "near" in its name, one would think it is near 1e100 (so a discounted 1e100 - get it now!). However, the main point is still that it is more numerical superstition than numerical analysis - and the reason for this discussion is just an error in one tool; taking time from more productive discussions.
To me this solution is sufficient.
Thank you @christiankral!
The general solution to this problem will be discussed elsewhere. Can we get an agreement on this local solution just for Spice3 and move on?