Opened 11 years ago

Closed 8 years ago

Last modified 8 years ago

#2250 closed enhancement (fixed)

Support DisplayUnit and unit conversion for parameter input in OMEdit

Reported by: casella Owned by: adeas31
Priority: blocker Milestone: 1.11.0
Component: OMEdit Version: trunk
Keywords: Cc: kofranek@…, bernt.lie@…

Description

The new MSL uses only SI units in the model, and assumes the GUI can handle DisplayUnits, as well as unit conversions for parameter input. This feature should be implemented in OMEdit in the next release.

Attachments (2)

OM_bug_displayunit.PNG (11.1 KB) - added by Christoph <buchner@…> 8 years ago.
plotting Screenshot
plot_display_units.png (522.8 KB) - added by spinnau@… 8 years ago.
wrong display units in plot

Download all attachments as: .zip

Change History (43)

comment:1 Changed 11 years ago by casella

  • Milestone changed from Future to 2.0.0

comment:2 Changed 11 years ago by sjoelund.se

r18156 added an API call that can convert between units (returns the scale factor and offset). Should be easy to add a displayUnit column to OMEdit (user editable, combo box that remembers the xml-file unit and displayUnit).

comment:3 Changed 10 years ago by casella

  • Priority changed from high to critical

comment:4 Changed 10 years ago by casella

  • Milestone changed from 2.0.0 to 1.9.2

comment:5 Changed 10 years ago by sjoelund.se

  • Milestone changed from 1.9.2 to 1.9.3

Milestone changed to 1.9.3 since 1.9.2 was released.

comment:6 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.3 to 1.9.4

Moved to new milestone 1.9.4

comment:7 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.4 to 1.9.5

Milestone pushed to 1.9.5

comment:8 Changed 9 years ago by sjoelund.se

  • Milestone changed from 1.9.5 to 1.10.0

Milestone renamed

comment:9 Changed 9 years ago by adeas31

Done in 5a75cb5/OMEdit.

comment:10 Changed 8 years ago by adeas31

  • Cc kofranek@… added

Changed 8 years ago by Christoph <buchner@…>

plotting Screenshot

comment:11 follow-up: Changed 8 years ago by Christoph <buchner@…>

@adeas31: The commit message says "The plot data is now based on the display unit.", but something must have gone wrong. The code

model Test_displayunit
Modelica.SIunits.Pressure p_test(displayUnit="bar");

equation
p_test = 5e5;
end Test_displayunit;

gives me the attached screenshot. Notice how the legend says [bar], but the data is still 5e5 (i.e. Pascal), not 5 (bar) as expected.

comment:12 Changed 8 years ago by Christoph <buchner@…>

Forgot to add, this is on OMEdit v1.10.0-dev-53-g45d32fb. I guess this needs to be reopened?

comment:13 follow-up: Changed 8 years ago by Christoph <buchner@…>

Also, the variable in the Variables Browser is also still in Pascal, so does not heed the displayUnits, although one might discuss which one it should be.
Personally, I think it should follow displayUnit, as that allows one to scale all the parameters of a model to look at them in one plot, e.g. a valve pressure [O(1e5-1e7)] plus a valve opening [0..1].

comment:14 in reply to: ↑ 13 Changed 8 years ago by casella

Replying to Christoph <buchner@…>:

Also, the variable in the Variables Browser is also still in Pascal

That's correct, since the unit is listed there (not the displayUnit).

so does not heed the displayUnits, although one might discuss which one it should be.
Personally, I think it should follow displayUnit, as that allows one to scale all the parameters of a model to look at them in one plot, e.g. a valve pressure [O(1e5-1e7)] plus a valve opening [0..1].

I don't think it is really necessary to list the displayUnits, only to handle them correctly when actually displaying something. I am not sure I understand what you mean with your comment regading the scaling of all parameters in one plot.

BTW, it should also be possible to change the displayUnit on the fly if one needs so, without changing the Modelica source and re-running the simulation.

comment:15 Changed 8 years ago by Christoph <buchner@…>

I am not sure I understand what you mean with your comment regading the scaling of all parameters in one plot.

Let's say you want to analyse the behaviour of a valve when varying the opening, and plot the upstream and downstream pressure, plus the opening. The pressures will typically be O(1e5-1e7), i.e. 1-100 bar, while the opening goes from 0..1. To meaningfully show this information together in one plot, you would set the displayUnit of the pressure to bar, so both quantities attain similar order of magnitude when analyzing.
As the plotting functionality currently does not honor displayUnit, all you see from the opening is a flat line on the bottom.
Alternatively, you can look at the variables browser, but when looking at the pressures you have to count zeros, which is error prone -- quick! is p=100000 or p=1000000 the value you expect? Again, it would be useful to honor the displayUnit the user set, to attain more similar magnitude for the quantities analyzed.

This gets worse the more quantities you plot, e.g. for the above valve you want to compare pressures (1e5-1e7 -> scale to bar), opening (0-1), and mass flow rates (1e-4 - 1e-2 -> scale to g/s).

comment:16 in reply to: ↑ 11 Changed 8 years ago by adeas31

Replying to Christoph <buchner@…>:

@adeas31: The commit message says "The plot data is now based on the display unit.", but something must have gone wrong. The code

model Test_displayunit
Modelica.SIunits.Pressure p_test(displayUnit="bar");

equation
p_test = 5e5;
end Test_displayunit;

gives me the attached screenshot. Notice how the legend says [bar], but the data is still 5e5 (i.e. Pascal), not 5 (bar) as expected.

Because we don't have bar as unit in our system. I have added it now in 4759770/OMCompiler/. However, OMEdit should display error if we don't support the display unit and should fallback to the original unit. I have fixed this in efb215b/OMEdit.

comment:17 Changed 8 years ago by Christoph <buchner@…>

Ah great, I will try this with the next nightly that has it.
Btw, regarding the quantity name, bar does not necessarily have to be "gauge pressure", it can be any kind of pressure (gauge, absolute,...), so it might be better to just say "pressure" to avoid confusion/misunderstandings. (Gauge pressure is commonly zero-referenced pressure against ambient air pressure)

comment:18 Changed 8 years ago by adeas31

comment:19 Changed 8 years ago by Christoph <buchner@…>

I can confirm that plotting now shows displayunit (bar). Thank you!

comment:20 follow-up: Changed 8 years ago by Filip Jezek

I just would like to inquire,

1) how to add units conversions - i.e. physiolibrary uses units common in medicine and comes with conversion scripts for dymola - see https://goo.gl/oQoNBj

2) adding the possibility to set the parameters in any compatible displayUnit would be a big up! (as it is in dymola). This enables to effectively approach the model in common units, although it is modeled (and thus compatible) in convenient SI units. Is it planned in this ticket, or in future one?

comment:21 in reply to: ↑ 20 Changed 8 years ago by adeas31

Replying to Filip Jezek:

I just would like to inquire,

1) how to add units conversions - i.e. physiolibrary uses units common in medicine and comes with conversion scripts for dymola - see https://goo.gl/oQoNBj

Right now we don't have anyway to add units. We have some basic units and derived units defined in OpenModelica.

2) adding the possibility to set the parameters in any compatible displayUnit would be a big up! (as it is in dymola). This enables to effectively approach the model in common units, although it is modeled (and thus compatible) in convenient SI units. Is it planned in this ticket, or in future one?

This is already done as part of this ticket. Try for example Temperature. You should be able to convert it from Kelvin to Celsius and vice versa.

comment:22 follow-up: Changed 8 years ago by casella

  • Cc bernt.lie@… added

Adeel, could you please add the units "hour" and "day" to the list of time units, and make sure it is also used for the plot scaling if set as displayUnit?

This requirement comes from Bernt Lie, which often needs multi-day simulations, where it is much better to see 1 day or 24 hrs rather than 8.4e6 seconds.

comment:23 Changed 8 years ago by adrpo

We should really be able to have this configurable so that anybody can add units to OpenModelica if they specify conversions wrt SI units.

comment:24 Changed 8 years ago by casella

That would be nice. A simple text file where each line contains a unit string, an SI unit string, the ratio and the offset w.r.t. SI unit should be enough.

comment:25 Changed 8 years ago by adrpo

It is basically like that in the C++ file:
https://github.com/OpenModelica/OMCompiler/blob/master/Compiler/runtime/unitparser.cpp#L1068
but we should be able to read additional ones in there.

comment:26 follow-up: Changed 8 years ago by Christoph <buchner@…>

Just found a small unrelated error in the file linked from the above comment: the megawatt radiant flux has a unit symbol of MA, not MW: https://github.com/OpenModelica/OMCompiler/blob/7c597ec8b277cbc6446344aefcfa9fb9e5fda848/Compiler/runtime/unitparser.cpp#L1128

comment:27 in reply to: ↑ 22 ; follow-up: Changed 8 years ago by adeas31

Replying to casella:

Adeel, could you please add the units "hour" and "day" to the list of time units, and make sure it is also used for the plot scaling if set as displayUnit?

This requirement comes from Bernt Lie, which often needs multi-day simulations, where it is much better to see 1 day or 24 hrs rather than 8.4e6 seconds.

Done in 58a8915/OMCompiler.
Note that plot can only display values accroding to the displayUnit. I still haven't implemented a way inside the plotting which allows you to choose from different units like in parameters window.

comment:28 in reply to: ↑ 26 Changed 8 years ago by adeas31

Replying to Christoph <buchner@…>:

Just found a small unrelated error in the file linked from the above comment: the megawatt radiant flux has a unit symbol of MA, not MW: https://github.com/OpenModelica/OMCompiler/blob/7c597ec8b277cbc6446344aefcfa9fb9e5fda848/Compiler/runtime/unitparser.cpp#L1128

Fixed in 58a8915/OMCompiler.

comment:29 in reply to: ↑ 27 Changed 8 years ago by casella

Replying to adeas31:

Note that plot can only display values accroding to the displayUnit. I still haven't implemented a way inside the plotting which allows you to choose from different units like in parameters window.

In fact, what Bernt was looking for was changing the time unit, i.e. the unit on the abscissa of the graph. When you allow to choose from different units for the Y axis, please remember to also make it possible for the X axis.

comment:30 follow-up: Changed 8 years ago by lochel

As far as I know, OpenModelica has still no unit information for built-in variable time. At least there is nothing in the variable browser of OMEdit.

comment:31 in reply to: ↑ 30 ; follow-up: Changed 8 years ago by casella

Replying to lochel:

As far as I know, OpenModelica has still no unit information for built-in variable time. At least there is nothing in the variable browser of OMEdit.

According to the Modelica specification, section 3.6.7, the built-in variable time has unit "s", so this needs to be fixed.

Unfortunately, I see no way to declare a displayUnit for time in a model, because time is only implicitly defined. However, OMEdit could give the option of setting the displayUnit of this built-in variable.

comment:32 in reply to: ↑ 31 Changed 8 years ago by sjoelund.se

Replying to casella:

Unfortunately, I see no way to declare a displayUnit for time in a model, because time is only implicitly defined. However, OMEdit could give the option of setting the displayUnit of this built-in variable.

OMEdit could give the option to change the displayUnit for any variable that is plotted, regardless if it was defined with a displayUnit or not :)

comment:33 Changed 8 years ago by adeas31

Yes, as long as the variable has unit defined.

comment:34 follow-up: Changed 8 years ago by adeas31

  • Resolution set to fixed
  • Status changed from new to closed

I added convert units support in plotting view as well a2b889d/OMEdit.
The variables browser now has display unit column which contains a drop down list with possible units.

comment:35 in reply to: ↑ 34 Changed 8 years ago by spinnau@…

Replying to adeas31:

I added convert units support in plotting view as well a2b889d/OMEdit.
The variables browser now has display unit column which contains a drop down list with possible units.

Great work. This really enhances the usability of OMEdit for the evaluation of simulation results. But the data is plotted with wrong units after re-simulation (please see screenshots attached):

  • Selecting the temperature variables after initial simulation will plot them correctly in degC
  • After re-simulation (without changing anything on the plot) one of the temperature curves will change to values in Kelvin. The display unit entry in the appropriate column in the Variable Browser remain unchanged
  • Changing the display unit setting to K for this variable gives totally wrong results in the plot, as 273.15 is added to the values that are already in K
  • Only solution to get the plot right again, is unchecking and re-checking this variable for plotting

Changed 8 years ago by spinnau@…

wrong display units in plot

comment:36 Changed 8 years ago by spinnau@…

  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopened, as the issue described above is closely related to the added units support in plotting view.

comment:37 Changed 8 years ago by Christoph <buchner@…>

I'm also seeing what spinnau reported.

comment:38 Changed 8 years ago by casella

  • Milestone changed from 1.10.0 to 2.0.0
  • Priority changed from critical to blocker

comment:39 follow-up: Changed 8 years ago by adeas31

  • Resolution set to fixed
  • Status changed from reopened to closed

Fixed in e4abcb5/OMEdit.

comment:40 in reply to: ↑ 39 Changed 8 years ago by spinnau@…

Many thanks to Adeel for the fix. The use case described in the screenshot in comment:36 now works as expected.

comment:41 Changed 8 years ago by sjoelund.se

  • Milestone changed from 2.0.0 to 1.11.0

1.11 will be released before 2.0

Note: See TracTickets for help on using tickets.