Add a block (based on time events) to calculate the extrema of a signal

[dietmarw]

Based on an old discussion MO#109 and now triggered again via @tinrabuzin.
The implementation is based on Modelica.Blocks.Math.Mean

I specifically put this into draft mode and like to discuss this first.

Some notes:

• I'm aware that there are some performance drawbacks, but these are the same as present in other block like Mean
• I'm not 100% happy with the name yet. Ideally I would have called it MinMax but that one is taken although one could argue that that block really should be placed under Blocks.Routing

[HansOlsson]

Looks good. Some comments:
I don't understand why there is both y_min_last and y_min (etc) to me they could be the same variable.
And I would add two warnings as comments:

• The extrema will be approximate as it only return the extremas at the sampling points.
• The sampling will generate many events which may slow down the simulation.

[dietmarw]

I've added some more documentation. and removed the intermediate discrete variables. Basically I was orienting myself on the implementation of the Mean block which probably also could get compacted in the same way.

[dietmarw]

I guess since it has already gone through one review it can probably get elevated from the draft status. Still open to suggestion regarding the name.

[christiankral]

I am very much in favor of the proposed min / max block. However, before adding my review here, I want to make to additional feature requests which are open for discussion:

1. When the min and max values of the input signal are caught, it sometimes makes sense to know at what (sampling) time the min / max values occurred. So it were nice if the two points in time could be recorded as variables in the block, too. In this case one can at least access this variable by an equation.
2. If I investigate an oscillation, I am sometimes also be interested, let's say in the second or third min / max value. In order to determine these values, different approaches are possible. Let me point out two options that came to my mind:
   1. Provide an option to the block to "reset" the min / max values. An example for such behavior is implemented in the integrator block.
   2. Make the output min / max values a vector and define a parameter n which defines the length of the two vectors. In this case n subsequent min / max values (and time instants, associated with option 1) are available.

[dietmarw]

I've added two additional discrete variables t_min and t_max to store the time of when the min and max was reached.
@HansOlsson I did use discrete type for those times though am now really unsure if that is necessary at all and/or gives any benefit over using continuous signals here.

@christiankral As for your other request number 2, I really like to keep this block in the way the other blocks work and also keep it simple, I can understand that the functionality you are looking for might come in handy but is probably quite special to your applications. So I'm reluctant to add this to this more simple block. Also this implementation is rather slow due to the sampling nature. So if you want a more sophisticated solution you might also look at https://github.com/beutlich/LastLib which does not generate ANY time events is hence much faster but depends on using the provided FMU (since the functionality was never accepted in Modelica Spec but is part of FMI spec).

[christiankral]

@dietmarw I fully understand your argument. That's OK with me.

[christiankral]

Alternative name proposal SampleMinMax

[christiankral]

Looks good

[dietmarw]

I don't think the name Sample(d)Something is a route to follow. A name of a component should be about what it does and not how it is achieving that. There are lots of sampled blocks (e.g., `Mean´) but we don't want to start renaming all those.

[AHaumer]

I don't think the name Sample(d)Something is a route to follow. A name of a component should be about what it does and not how it is achieving that. There are lots of sampled blocks (e.g., `Mean´) but we don't want to start renaming all those.

To be honest: The basic definitions of RMS and similar are sampled per basic period.
That's not the case for min / max - which have to be sampled at higher frequency.
I'm searching for a min / max detection without sampling - didn't find a solution up to now, maybe never ...
But if, how would you name that? ContinuousMinMax?
I'd prefer SampledMinMax.

[AHaumer]

Well I know that's the best solution we have (since we have no "continuous" implementation).
I think you should include a bold warning to set the sampleTime appropriately, and include 2 usages for the same signal: 1 with bad (too large) choice of sampleTime, 1 with sufficiently small sampleTime.
BTW, you forgot to intialize t_min and t_max ...
I've committed my suggestions for an example and the initialization of t_min and t_max.

[AHaumer]

Fine with me!

[AHaumer]

What's wrong with the CI/syntax_checks?

[AHaumer]

Sorry when adding comparisonSignals.txt, I realized that's not reasonable using random signals.
Therefore I adapted the example, then added comparisonSignals.txt.

[AHaumer]

after adapting the example, it's fine for me.

[HansOlsson]

What's wrong with the CI/syntax_checks?

It seems the file starts with a UTF8-Byte Order Mark and the CI-check doesn't like that.
If you are using Dymola I think you can avoid that by setting Hidden.SupportUnicodeFiles=3; but we will clean up that.

[HansOlsson]

Looks good now.

[dietmarw]

Nice example @AHaumer. With this example we can actually remove the test one I had added to ModelicaTest. No point in maintaining two models. So I will remove that one again .

[dietmarw]

Also fine by me now. One of the library officers should merge this then.

[christiankral]

Looks good.

[christiankral]

Sorry for merging... I'm not a library officer.

[beutlich]

I'm searching for a min / max detection without sampling - didn't find a solution up to now, maybe never ...
[...]
Well I know that's the best solution we have (since we have no "continuous" implementation).

@AHaumer Have a look at the FMU-based solution MinimumTest.Minimum block, which provides a continuous implementation. It is legal Modelica but the FMU import is tool-specific.

https://github.com/beutlich/LastLib/blob/543d88ecf891796640727f126250fe69a3c41549/MinimumTest.mo#L4-L21

See also https://tinyurl.com/lastlibfmu for some background information and the necessary tool-specific workarounds. The FMU (called Last.fmu) was recently reimplemented (FMI 1,0 ME) and is successfully used for parameter optimization tasks.

[christiankral]

@dietmarw I have two questions:

1. Is there a particular reason for the parameter names

   • Ts instead of samplePeriod and
   • t0 instead of startTime?
     The parameter names samplePeriod and startTime are consistently used in the package Modelica.Blocks.Discrete for sampling blocks.

2. As you suggested to use the name MinMax I wonder if you think that it makes sense to move / rename the model Modelica.Blocks.Math.SignalExtrema to Modelica.Blocks.Discrete.MinMax. You might have thought about it before, but just want to make sure to explicitly ask this question.

As we also have a a continuous and a discrete implementation of the models TransferFunction

it would also make sense to have Modelica.Blocks.Continuous.MinMax one day in the future...

[dietmarw]

@christiankral Thanks for the feedback. I had not considered putting this block inside Modelica.Blocks.Discrete before and hence was not aware of the nomenclature as used in there. Thinking about it, it might actually be a good solution placing it there as it then also emphasises the "sampled" nature plus I get the choose my preferred name MinMax. In that case I would also rename Ts and t0.
@AHaumer any objections?

As for the future perspective of one day having a non-sampled version of MinMax (as demonstrated by https://github.com/beutlich/LastLib) I think we also have the problem of clearly establishing what goes into Blocks.Continuous and what in Blocks.Math. At the moment it looks rather random. But that would be for a totally different (and low priority) ticket.