Use boolean array reductions - #3919
Harisankar-Allimangalath merged 6 commits into
Conversation
This takes advantages of tools' dedicated implementations of conjunction and disjunction of multiple values expressed as Boolean array reduction with min and max, respectively. With the result being given by simple expressions, it seemed natural to also set Inline = true.
|
@henrikt-ma Hmmm ... tried your solution with Dymola 2021x: |
|
Just a less elegant solution but also a one-liner (maybe intuitive to read due to recursion): |
It's valid at least as of Modelica 3.4: https://specification.modelica.org/maint/3.4/Ch10.html#reduction-expressions |
I don't think a recursive implementation has any clear advantage over the old algorithm with iteration. The point or the PR is to make use of the simplicity and efficiency of array reductions. (By the way, if one should make a recursive implementation, I think that the base case should be the empty array.) |
|
Yes, I agree that |
Of the two alternatives, I'd prefer waiting for them to get the support in place. I see a clear risk of the reduction with iterator not being implemented as efficiently as the variant that looks like a function call. |
It will be fully implemented in Dymola 2023. I don't understand why it was missed earlier. |
HansOlsson
left a comment
There was a problem hiding this comment.
The code is good, but as noted it would be preferable for us to wait until Dymola 2023 is released (well, or at least about to be released).
Addressing review comment by @beutlich.
As suggested by @beutlich.
The motivation is twofold: Tools may have efficient implementations for the |
|
@HansOlsson, I suppose Dymola 2023 has been released by now, so that we can proceed here? |
|
@HansOlsson @MartinOtter, can this approved PR be labeled with 4.1.0 milestone? |
|
I'd say, if ModelicaTest.Math.BooleanFunctions still succeeds it can be merged (with milestone set accordingly). |
Yes. |
yes the test still succeeds , and the reference files are matching |
Opening this based on recent comment by @AHaumer.
The point of the change is to take advantage of optimized tool implementations of Boolean array reductions.