Niac changes for apm, em, cg, and ms brought back to fairmat assuming that these will be accepted - #353
Conversation
… probe app, we have edited these here, despite there is currently an active NIAC-feature branch fairmat-2024-apm that is in the process of negotiation with NIAC. To move forward we still work with the old fairmat branch, knowing that the example will need refactoring and reprocessing ones NXapm has been accepted as an official appdef in NeXus by the NIAC, likely end of Q1 2025
…nyaml addressing proper handling of copyright dates as discussed in e.g. nexusformat#1303 on the niac side
… be used only in atom probe and thus may not warrant to become proposed towards NIAC
… likely be used only in atom probe and thus may not warrant to become proposed towards NIAC
…ed strong enough and thus may not warrant to become proposed towards NIAC
…strong enough but used in the end only by NXapm thus we thought it is better to move it into the appdef directly, also because NXapm can cover now use cases of measurements and is envisioned to cover also simulations of APT and FIM so no more need to warrant another appdef that would then use NXapm_hit_finding, cameca could inherit from NXapm as base class inheritance has already been accepted by the NIAC as a result of the Autumn NIAC 2024 hackathon
…nly a container to store instances of NXevent_data_em, reorganized via defining these concepts directly in NXapm_msr
…definition below instrument that was required with the previous design. The key idea always was to have one group where static props of the instrument are stored and another group (event_data) where dynamic values are stored, so far the implementation for this recreated all concepts of the instrument on the one hand in NXapm_msr and on the other hand in NXevent_data_apm. However, now with base class inheritance supported and accepted by the NIAC, it would be much better to refactor NXapm_msr towards a base class called NXinstrument_apm that offers a dictionary for the concepts for static and typically quantities with more dynamic i.e. frequently changing values. Thereby measurement and NXevent_data_apm can just inherit NXinstrument_apm and thus the description of the NXevent_data_apm base class becomes much slimmer, less prone to making manual errors like copy-paste, the appdef will then just constrain for which concepts inherited from either NXinstrument_apm will be required, then possibly one may even remove NXevent_data_apm altogether via eventID(NXobject) with a child NXinstrument_apm
…msr refactored into NXinstrument_apm, next and last step, check that relevant groups of NXinstrument_apm are defined and constrained properly in the NXapm appdef to complete the BREAKING REFACTORING
…iles from CompositionSpace (NFDI-MatWerk) or paraprobe-toolbox (FAIRmat) into NOMAD for the apmapp developments
…ts and generate NXDL files
There was a problem hiding this comment.
So far, the strategy was that we don't move anything to base_classes or applications yet that is not accepted, but to keep everything in contributed_definitions for now (see #329, #338, #349, #350). In this PR, basically everything is moved to these folders. This will be a problem because we are using similar base classes (like NXpeak) and we will have them both in base_classes and contributed_definitions now, which makes it imposssible to see if there are any changes. I suggest to move these files back to contributed until they are accepted by NIAC.
There are also some dev_tools and manual files that need to be reverted back to the state in fairmat, I left comments wherever I detected these.
I have not yet checked any of the actual NXDL files, will do after moving.
…ese have not been accepted by the NIAC yet
… FAIRmat-NFDI/project sprintboard issue 104 feedback from NFFA folks
… FAIRmat-NFDI/project sprintboard issue 104 feedback from NFFA folks, 2
…he current fairmat branch
…ng, this branch is feature complete
lukaspie
left a comment
There was a problem hiding this comment.
LGTM. I checked all files, but only ran though them quickly.
The copyrights are significantly changed in many places and are often wrong now, but this can be handled in a follow-up PR.
This PR consolidates the work on definitions for the APM and EM apps and brings back in this process the NIAC changes for the follow sets of definitions:
NXapm* --> assuming acceptance of fairmat-2024-apm
NXem* --> assuming acceptance fairmat-2024-em
NXcg* with supportive NXcs* --> fairmat-2024-computational-geometry
Assuming acceptance of NIAC non-FAIRmat geometry updates 1532
NXmicrostructure* just updated planned to stay in contributed_definitions
NXem_calorimetry just updated planned to stay in contributed_definitions
After the fairmat branch was merged in all yaml files were recreated firstly via make clean, make nyaml, make nxdl
Fixes of warnings in the documentation building except for these ...
despite remaining issues review of this PR should start asap cuz this PR is required for composing a consolidated fairmat branch to update pynxtools, I will work on the next issue in parallel
... remaining issues, sorted in order of decreasing implementation priority:
Comments from Lukas,
Furthermore,
Further issues remaining that should not block though this PR and on which we should not start working before apm and em examples are working with pynxtools and with examples that work in apps (again):