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
452 stars 165 forks source link

Update MultipleResonance.mo #4351

Closed hubertus65 closed 3 months ago

hubertus65 commented 4 months ago

Fixed to make type more clear, and make it work in Modelon Impact. Also, removed an unused line of code. This is tested in latest version of Modelon Impact, and Dymola, and works in both.

HansOlsson commented 3 months ago

This does not work in Dymola 2024x, and I cannot see how it would work according to the specification. It may have worked in earlier versions (depending on flags) where we were a bit too aggressive in optimizing multiplication by zero.

Longer explanation for why: Modelica.Units.SI.ComplexImpedance is just a Complex with units added - and should thus be seen as a constructor call according to:

https://specification.modelica.org/master/overloaded-operators.html#overloaded-constructors

There is only one constructor-function in Complex - fromReal where the imaginary part has default-value 0; so the only match we consider would be: Complex.'constructor'.fromReal(re=100+Complex.j*0, im=0) which fails as the first argument is Complex and not real. Having "copy-constructor" for types may be an interesting idea, but it hasn't been added yet.

How to correct:

However, Modelica.Units.SI.ComplexImpedance(100) or Modelica.Units.SI.ComplexImpedance(100, 0) would be valid.

Harisankar-Allimangalath commented 3 months ago

@hubertus65 @AHaumer can you please look in to the above suggestion and decide on how to proceed with this .

Thankyou

beutlich commented 3 months ago

How to correct:

Addressed by #4374.