ModelicaStandardLibrary icon indicating copy to clipboard operation
ModelicaStandardLibrary copied to clipboard
verified publisher icon modelica

AdditionalSqrt

Open Esther-Devakirubai opened this issue 1 year ago • 6 comments

Backport #4396

Esther-Devakirubai avatar Aug 16 '24 03:08 Esther-Devakirubai

@Esther-Devakirubai #4396 affects four files, while this backport only affects two. I guess there are some additional commits missing

Will check.

Esther-Devakirubai avatar Aug 19 '24 16:08 Esther-Devakirubai

@Esther-Devakirubai #4396 affects four files, while this backport only affects two. I guess there are some additional commits missing

Will check.

@casella I see only 2 files being affected by this.

https://github.com/modelica/ModelicaStandardLibrary/commit/1be54b5edfd3370f0365cab256cbed62c40bc2bc

Please correct me if I am wrong.

Esther-Devakirubai avatar Sep 04 '24 09:09 Esther-Devakirubai

@Esther-Devakirubai there are more commits in #4396 : https://github.com/modelica/ModelicaStandardLibrary/pull/4396/commits

but this PR only contains the contents of one of them.

maltelenz avatar Sep 09 '24 06:09 maltelenz

@Esther-Devakirubai there are more commits in #4396 : https://github.com/modelica/ModelicaStandardLibrary/pull/4396/commits

but this PR only contains the contents of one of them.

Yes, the #4396 PR contains multple commits, but for some reason the cherry-picking only considered the first one

casella avatar Sep 17 '24 09:09 casella

I don't get why .sqrt and not just sqrt. sqrt(v) is defined as the square root if v is Real or Integer, which is the case here. @HansOlsson, why the leading dot?

There is a comment explaining that on the line: "Real, not complex sqrt".

Due to lookup sqrt in Modelica.ComplexMath will find the complex Modelica.ComplexMath.sqrt-function, which isn't desired here. The complex exp and log functions work around it by using Modelica.Math.exp and Modelica.Math.log

HansOlsson avatar Oct 18 '24 07:10 HansOlsson

@casella Can this be approved and Merged?

Esther-Devakirubai avatar Oct 22 '24 04:10 Esther-Devakirubai

There is a comment explaining that on the line: "Real, not complex sqrt".

Due to lookup sqrt in Modelica.ComplexMath will find the complex Modelica.ComplexMath.sqrt-function, which isn't desired here. The complex exp and log functions work around it by using Modelica.Math.exp and Modelica.Math.log

I previously assumed that built-in functions had precedence in the look-up process, so they could not be shadowed. I obviously missed Section 5.6.1.1 saying: "The builtin classes are put into the unnamed root of the class tree".

casella avatar Oct 24 '24 13:10 casella

@casella Can this be approved and Merged?

Yes, please go ahead. Thanks!

casella avatar Oct 24 '24 13:10 casella

@casella Can this be approved and Merged?

Yes, please go ahead. Thanks!

@casella You requested changes earlier, so you have to actually approve :)

maltelenz avatar Oct 24 '24 13:10 maltelenz

@casella You requested changes earlier, so you have to actually approve :)

😅

casella avatar Oct 24 '24 15:10 casella