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

Add Spice3-specific infiniteTime constant #4481

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions Modelica/Electrical/Spice3.mo
Original file line number Diff line number Diff line change
Expand Up @@ -3134,8 +3134,8 @@ Christoph Clauß
parameter SI.Time TD = 0.0 "Delay time";
parameter SI.Time TR(start=1) "Rise time";
parameter SI.Time TF = TR "Fall time";
parameter SI.Time PW = Modelica.Constants.inf "Pulse width";
parameter SI.Time PER= Modelica.Constants.inf "Period";
parameter SI.Time PW = Spice3.Constants.infiniteTime "Pulse width";
parameter SI.Time PER= Spice3.Constants.infiniteTime "Period";

protected
parameter SI.Time Trising=TR "End time of rising phase within one period";
Expand Down Expand Up @@ -3512,8 +3512,8 @@ If, e.g., time = 1.0, the voltage v = 0.0 (before event), 1.0 (after event)
parameter SI.Time TD = 0.0 "Delay time";
parameter SI.Time TR(start=1) "Rise time";
parameter SI.Time TF = TR "Fall time";
parameter SI.Time PW = Modelica.Constants.inf "Pulse width";
parameter SI.Time PER= Modelica.Constants.inf "Period";
parameter SI.Time PW = Spice3.Constants.infiniteTime "Pulse width";
parameter SI.Time PER= Spice3.Constants.infiniteTime "Period";

protected
parameter SI.Time Trising=TR "End time of rising phase within one period";
Expand Down Expand Up @@ -4435,6 +4435,12 @@ on the model behaviour.
</html>"));
end Types;

package Constants "Spice3-specific constants"
extends Modelica.Icons.Package;

constant Modelica.Units.SI.Time infiniteTime= 1e20 "Very large time horizon (3e12 years) avoiding overflow in time computations";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we are defining new (local) constants, what do you think about not enlarging the scope of the problem of the unfortunate (incorrect) naming of Modelica.Constants.inf, and give this constant a more accurate name, e.g. hugeTime or largeTime or something like that, instead of infinity, which it is not?

Copy link
Contributor Author

@casella casella Oct 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's infinite for all practical purposes. Large or huge have no definite meaning, large with respect to what? Infinite time period and width means you'll never see a falling edge, so you'll get the rising edge only.

If you look at the parameter window and you see that both period and width are "infinite" it is clear that there will be no periodic behaviour if you leave those parameters to their default values. Which I guess was the original intent of the authors.

The problem with using Modelica.Constants.inf here is not in the name, it's in the value.

But I guess we are over-discussing this issue here...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then.

end Constants;

package Internal
"Collection of functions and records derived from the C++ Spice library"
extends Modelica.Icons.InternalPackage;
Expand Down