open-simulation-platform / libcosim

OSP C++ co-simulation library
https://open-simulation-platform.github.io/libcosim
Mozilla Public License 2.0
61 stars 10 forks source link

Signal name when logging signals to csv file #712

Closed tomarnepedersen closed 1 year ago

tomarnepedersen commented 2 years ago

When signals are logged to csv file, the naming convention seem to be: [signal name] [[signal id] [signal type] e.g. azimuth_thrust [6 real output]

When new signals are added to the model, the id may change, even though the name is the same. So after the model has been updated and compiled to fmu, the signal could have gotten a new id, which gives e.g. the following signal name in the csv file: azimuth_thrust [8 real output]

Then, when reading the csv file, and plotting signals, the code for plotting will need to be updated since the ID has changed. I would suggest to not include the ID in the signal name when logging to file.

eidekrist commented 2 years ago

Moved to libcosim and added "discussion needed" label.

Has anyone ever used the additional metadata ([valuereference type causality]) for anything?

markaren commented 2 years ago

In the very specific cases where this would be an issue, one could just ignore the metadata in the plotter. This sounds like an easy to fix issue in the plotting code.

Altough, there probably isn't that much value in keeping valuereference as you'd probably search using the name anyway.

kyllingstad commented 1 year ago

I think we should drop the additional metadata from the header line and just have the variable name there. In the few cases where such information is needed, it can be gotten from elsewhere.

In the very specific cases where this would be an issue, one could just ignore the metadata in the plotter.

This presumes that the user has programmatic control of the plotting. But I'd argue that's not the typical case; most will use point-and-click solutions (e.g. Excel).

restenb commented 1 year ago

This has (hopefully) been covered by https://github.com/open-simulation-platform/libcosim/pull/725 in the latest release, so I'm closing this.