attach monitoring data to hdf5 file

[TjarkMiener]

This PR tweaks the merger component to add functionalities listed in #2900 (comment)

closes #2890

[TjarkMiener]

Back to draft mode since new test files will be uploaded soon. Requires some updates then.

[TjarkMiener]

Should be rebase to #2881

[TjarkMiener]

Should be rebase to #2881

With the data format version bump of #2881 (and the additional here), the tests are failing because the interface i0.2.0 test data were created with data format v7.2.0. I will re-create with the latest version, but it is raising a crucial point: We need to allow attaching of monitoring from different data format versions, in order not to break the CI with the interface file when (unrelated) data format changes results in a bump of version.

[maxnoe]

As far as I can tell, the CI failure has nothing to do with data format versions.

And this also should never be the case, as long as we support the older versions (which we should, to some point).

[maxnoe]

E.g. this is simply a bug:

>               f"Input file {other.filename:!r} has different data model version:"
                             ^^^^^^^^^^^^^^^^^^^
                f" {other_version}, expected {self.data_model_version}"
            )
E           ValueError: Invalid format specifier '!r' for object of type 'str'

The colon is wrong here, it needs to be {other.filename!r} not {other.filename:!r}

[TjarkMiener]

I needed to give some thoughts about the different options in the merger component. As I see it now we have three main usecases to cover with this component:

1. Merging of simulation, where we have many obs_id
2. Merging of chunks in the same observational block
3. Attaching of monitoring data to already merged products

This implies the following:

1. Process tool should not attach monitoring data to the resulting HDF5 file.
2. monitoring trait is redundant should be removed because monitoring data should only be attached via attach_monitoring trait flag.
3. single-ob and attach_monitoring can not be True at the same time and should result in a TraitError.
4. attach_monitoring can only be True if we are in appending mode (i.e. output file needs to exist)

@maxnoe @mexanick please let me know if you agree so I can add this logic to the option of this component.

[TjarkMiener]

From my side this PR would be ready for review, we just had an internal discussion yesterday if we should allow or forbid merging monitoring data to the event data. What are your thoughts on this, @maxnoe @kosack? In the current version of this PR, we allow this behaviour.

EDIT:

Yes, events files should be able to contain monitoring data.

Answered by @maxnoe in discussion above

[mexanick]

LGTM

@maxnoe I think the only remaining open topic is considering the check whether the file contains simulation data. I would prefer to use metadata for it and not the negative event IDs. Re-sonar complexity complain, I'd accept it.

[maxnoe]

Yes, metadata is certainly better than the event ids, which shouldn't even exist, it's a very bad workaround due to a simtel limitation of not assigning unique event ids to calibration events.

[mexanick]

@maxnoe @kosack could you please review? This PR is highly desirable for the upcoming DPPS release.

[maxnoe]

The only thing I see missing is the resolution of the attach_monitoring vs. monitoring discussion.

I thought we agreed to replace attach_monitoring by something like: file_type = CaseLessStrEenum(["events", "monitoring"])?

[maxnoe]

You can keep this flag (and add one for the monitoring merging) by doing:

        "single-ob": (
            {"HDF5Merger": {"merge_strategy":  "events-single-ob"}},
            (
                "By default, the merge tool assumes it is merging multiple"
                " observation blocks. This option switches to merging multiple"
                " chunks of events of the same ob."
            ),
        )

[TjarkMiener]

Sounds good! Done in 3a71ef0.

[ctao-sonarqube]

Quality Gate passed

Issues
0 New issues
1 Fixed issue
1 Accepted issue

Measures
0 Security Hotspots
94.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube