Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fairmat 2024: several new base classes in NXsample and NXsample_component #1413

Open
wants to merge 37 commits into
base: main
Choose a base branch
from

Conversation

lukaspie
Copy link
Contributor

@lukaspie lukaspie commented Sep 23, 2024

This PR is aimed at a more detailed description of the components of a sample. It introduces the following new base classes:
NXsample_component_set, NXsubstance, NXunit_cell, NXsingle_crystal, NXrotation_set

- NXsample_component_set: this is meant as a wrapper that takes in all the sub sample components (i.e. instances of NXsample_component) and their relationships (mass fractions, volume fractions, etc.)
- NXsubstance: this is meant to describe a chemical substance. Examples can be single chemical elements, chemical compunds, or alloys (e.g. CuO, C,). It is envisioned that if such a substance is crystalline, it could have an instance of
- NXunit_cell: Description of a unit cell, i.e., the crystal structure of a single thermodynamic phase. This was can be used within NXsample_(component) and/or in
- NXsingle_crystal: this is for describing a single crystalline material/phase within another material. It makes use of the new base_class NXrotation_set, which provides a rigorous way of describing rotations, orientations, and disorientations.

Eventually, it is envisioned that each sample can contain one or more instances NXsample_component_set, each of which has one or more instances of NXsample_component. This component is either not mixed (and thus can contain an instance of NXsubstance) or it by itself contains one or more instances NXsample_component_set with multiple instances of NXsample_component. The image provides a useful visualization of this nesting.

This PR was originally aimed at a more detailed description of the components of a sample (see crossed out text above). However, we realized in the meantime that this is not yet stable and more (internal) discussion is needed. We will bring this up at a later point (likely for the release after the next one) again.

Thus, this PR is much shorter than initially. It brings in NXhistory and NXactivity (as already approved in #1419) to NXsample, to describe the history of physical activities that have happened on the sample (preparation, calcination, in situ treatment, ...). It also add NXsubstance (meant to describe a "pure" chemical substance) to the contributed_definitions.

@lukaspie lukaspie marked this pull request as draft September 23, 2024 15:59
@lukaspie lukaspie force-pushed the fairmat-2024-nxsample branch from 07d88ba to da28e00 Compare September 23, 2024 17:02
@lukaspie lukaspie changed the title Fairmat 2024: introduce several new base classes to be used in NXsample and NXsample_component Fairmat 2024: several new base classes in NXsample and NXsample_component Sep 24, 2024
@lukaspie lukaspie force-pushed the fairmat-2024-nxsample branch from e123e88 to cdb9b96 Compare September 24, 2024 07:04
@lukaspie lukaspie marked this pull request as ready for review September 24, 2024 07:11
This was referenced Sep 29, 2024
This was linked to issues Sep 29, 2024
@lukaspie
Copy link
Contributor Author

lukaspie commented Oct 7, 2024

Dev notes to keep track:

ToDo:
- NXsubstance: uses identifier -> blocked by discussion in #1486

@phyy-nx
Copy link
Contributor

phyy-nx commented Jan 27, 2025

From Telco:

@lukaspie
Copy link
Contributor Author

From Telco:

  • Remove NXfabrication as it's in NXcomponent

Done!

@phyy-nx
Copy link
Contributor

phyy-nx commented Jan 27, 2025

Hi all, as discussed in the Telco, we can now move this PR to an online vote. NIAC committee members please vote on this PR using emojis on this comment. 👍 for yes, 👎 for no, anything else (for example 👀) to abstain. We need 14 votes to hit quorum so please review and vote!

(note that NXsubstance does not need a vote as it's a contributed definition and note that NXhistory and NXactivity are part of #1419 which is due to be merged)

@lukaspie
Copy link
Contributor Author

Had to run black on dev_tools/tests/test_nxdl_utils.py to get the CI to pass.

lukaspie and others added 28 commits February 7, 2025 09:25
… version of yaml.

Removing unintensional comments
Since we have substantial changes on the NXsample base class now, here we remove any changes to NXsample in this PR.
# Conflicts:
#	contributed_definitions/NXmpes.nxdl.xml
#	contributed_definitions/nyaml/NXmpes.yaml
# Conflicts:
#	base_classes/NXsample_component.nxdl.xml
#	base_classes/nyaml/NXsample.yaml
#	base_classes/nyaml/NXsample_component.yaml
# Conflicts:
#	base_classes/NXsample_component.nxdl.xml
#	base_classes/nyaml/NXsample.yaml
#	base_classes/nyaml/NXsample_component.yaml
# Conflicts:
#	base_classes/nyaml/NXsample.yaml
# Conflicts:
#	base_classes/NXuser.nxdl.xml
#	base_classes/nyaml/NXsample.yaml
#	base_classes/nyaml/NXuser.yaml
#	contributed_definitions/NXapm.nxdl.xml
#	contributed_definitions/NXfabrication.nxdl.xml
#	contributed_definitions/NXidentifier.nxdl.xml
#	contributed_definitions/NXoptical_spectroscopy.nxdl.xml
#	contributed_definitions/nyaml/NXapm.yaml
#	contributed_definitions/nyaml/NXfabrication.yaml
#	contributed_definitions/nyaml/NXidentifier.yaml
#	contributed_definitions/nyaml/NXoptical_spectroscopy.yaml
# Conflicts:
#	base_classes/NXactivity.nxdl.xml
#	contributed_definitions/NXactivity.nxdl.xml
#	contributed_definitions/NXaperture_em.nxdl.xml
#	contributed_definitions/NXchemical_process.nxdl.xml
#	contributed_definitions/NXelectronanalyser.nxdl.xml
#	contributed_definitions/NXrotation_set.nxdl.xml
#	contributed_definitions/NXunit_cell.nxdl.xml
pull in cited base classes

pull in cited base classes
@lukaspie lukaspie force-pushed the fairmat-2024-nxsample branch from 35e3b89 to 6180f23 Compare February 7, 2025 08:27
@lukaspie
Copy link
Contributor Author

lukaspie commented Feb 7, 2025

Rebased after merging #1419, so NXactivity and NXhistory are no longer part of this PR. Comments above should then become issues if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion needed NIAC vote needed PR needs an approving vote from NIAC before merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NXsample NXsample_component
8 participants