search

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
472 stars 169 forks source link

Declare tolerance as MSL constant #4277

Open tobolar opened 8 months ago

tobolar commented 8 months ago

Close #4193

tobolar commented 8 months ago

The change from 1e-4 to Modelica.Constants.eps is massive. I made a small test with System Modeler, and to my big surprise the asserts were not triggered, despite the new tolerance being extremely tight.

This is how I understand the examples:

  1. Elements having no number in names (e.g. damper or prismatic) and prismatic.stateSelect=StateSelect.never.
  2. Elements with "index" 1 (e.g. damper1). Damper has activated heat connector and prismatic.stateSelect=StateSelect.always.
  3. Elements with "index" 2 (e.g. damper2). Here, prismatic2 has fixed initial acceleration <> 0 and damper2 has set only initial value of the damping parameter d (i.e. damper2(d(fixed=false, start=3))). So the tool has to calculate the value of d based on the parameters and initial conditions.

Basically, all three groups have parameters and initial contitions set in a way to deliver numercally identical results - which is the case for Dymola. So I'm happy to hear System Modeler performs the same way.

I hope that someone with a deeper understanding of the dynamics here can approve the change to such tight tolerances.

I hope so, too.

What about all the other test models in ModelicaTest.MultiBody that still use 1e-4?

Hmm, they are quite a lot. :-( I can try to look over the next days/weeks.

tobolar commented 8 months ago

Several different modeling ways are compared in a simulation. Signals are computed in the order of the relative tolerance, so the correct way would be something like tol = 10*relativeTolerance. However, this cannot be expressed in Modelica. 1e-4 is far good enough as an approximation

Ok. The tolerance is set again to 1e-4.

I will change the tolerances back to 1e-4 also in https://github.com/modelica/ModelicaStandardLibrary/pull/4276

tobolar commented 8 months ago

There are now lots of unrelated changes in graphic coordinates. Please remove.

Sorry for this. Fixed now.

henrikt-ma commented 8 months ago

Thus to me it would make sense to instead have two tolerances in the model: rtol (position) and vtol (velocity), even if they have the same value.

Sounds like a good alternative.

tobolar commented 8 months ago

Thus to me it would make sense to instead have two tolerances in the model: rtol (position) and vtol (velocity), even if they have the same value.

I can change this, retaining the same values. Reasonable also to give them units (position, velocity)?

@MartinOtter Do you agree?

As this changes the original idea of this PR, I guess we can close this PR without merging. The introduction of rtol, vtol shall IMO be done after #4276 (which fixes the assert-clauses in general) or as a part of it.

henrikt-ma commented 8 months ago

I can change this, retaining the same values. Reasonable also to give them units (position, velocity)?

Yes, this will ensure unit-awareness when viewing/modifying the parameters in context where units determined by inference are not available.