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 168 forks source link

Inconsistent use of `tol` in `ModelicaTest.MultiBody.Forces.Damper` #4193

Open qlambert-pro opened 1 year ago

qlambert-pro commented 1 year ago

The variable used as tolerance margin to decide whether two Real values are equal is used to compare position and speed at the same time.

We could turn tol into a constant as we don't expect the infer the unit of constants based on their usage or maybe use Modelica.Constants.eps?

qlambert-pro commented 1 year ago

Same issue in ModelicaTest.MultiBody.Forces.Damper2, ModelicaTest.MultiBody.Forces.Spring and ModelicaTest.MultiBody.Forces.Spring2

tobolar commented 8 months ago

I don't understand the point of the issue. Can you explain in more detail?

Regarding ModelicaTest.MultiBody.Forces.Spring and ModelicaTest.MultiBody.Forces.Spring2, there is a mess in using toland assert. I will prepare a fix.

henrikt-ma commented 8 months ago

This has to do with unit checking. When unit inference is applied, the tol should get an inferred unit which is consistent with how it is used, but this gets problematic if it is used inconsistently unit-wise. What @qlambert-pro said about using a constant instead of a parameter has to do with the current idea that unit inference in Modelica should not be allowed for constants, so that a constant with empty unit will behave just as a literal number.

tobolar commented 8 months ago

@henrikt-ma Many thanks. I didn't fully follow the unit discussion.

Then I guess both suggestions (changing to constant and equal to Modelica.Constants.eps) are fine. (At least Dymola didn't indicated any issue.)