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

Add nexus definitions/files for beam path description #183

Merged

Conversation

RonHildebrandt
Copy link

Adding functionality for a description of a beam path
In total four NeXus definitions are added:

  • NXopt_assembly
  • NXopt_beam
  • NXopt_element
  • NXtransfer_matrix_table

Optical elements (NXopt_element) describe elements of a setup. A beam can be defined as object(NXopt_beam), which is associated with two optical elements and can be used to describe physical properties of a photon beam.
A unique path can describe the beam properties along a starting element (source) and an ending element (detector). The beam properties are modified by each optical element (e.g. attenuation, divergence). Matrices shall be used to describe this change of beam properties. These elements are part of the optical assemly (NXopt_assembly), in which the respective formalism is given.
For each formalism, there can be different matrices. These matricies are part of a large table, which contains all the necessary information to model the beam properties along a path. Each matrix table is associated for one optical element for one combination of input and output beams.

Unclear
The NXtransfer_matrix_table data structure is based on the NXellipsometry definition. Im quite unsure if this structure can be functional this way.

First PR
As this is the first PR by myself - Any advices for further PR and explanations for general rules are welcomed
Just did by to the best of my knowledge and belief.

Copy link
Collaborator

@lukaspie lukaspie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @RonHildebrandt, congrats on your first PR 😄 I wasn't sure if you wanted to have review yet, but this wasn't marked as draft. If you want to ask specific people to review, you can tag them in the upper right corner.

I have added some comments, mostly related to wording. However, there are also some design flaws:

  • You need to define your base classes properly, i.e., as NXmy_base_class(NXobject).
  • You need to define dimensions symbols.
  • At the moment, you cannot extend another base class. This is what we have called base class inheritance, which was not in the original NeXus design. We will probably have this at some point, but for now, you must extend NXobject. If you want to extend NXbeam (and bring all of its fields and attributes), you have to manually copy all of them for now.

In general, all of these files should be in the contributed_definitions/nyaml folder because they are not accepted by NIAC for now. In addition, you should also make the NXDL files (by make nxdl) and make sure NXDL and NYAML are consistent.

base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_beam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_element.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXopt_element.yaml Outdated Show resolved Hide resolved
@lukaspie lukaspie marked this pull request as draft February 28, 2024 13:36
@RonHildebrandt
Copy link
Author

I have added adjustments of all files.
Still have to solve the failing checks. Is there a problem with the nyaml versions right now or should it be fine, if "make local" and "make nxdl" works?

Ron Hildebrandt added 2 commits March 6, 2024 11:58
@sanbrock sanbrock marked this pull request as ready for review March 6, 2024 11:46
Copy link

@sanbrock sanbrock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sanbrock sanbrock merged commit ffc7196 into FAIRmat-NFDI:fairmat Mar 6, 2024
1 of 6 checks passed
lukaspie pushed a commit that referenced this pull request Sep 23, 2024
* Add nexus definitions/files for beam path description

* Update base_classes/nyaml/NXopt_assembly.yaml

Co-authored-by: Lukas Pielsticker <[email protected]>

* Update base_classes/nyaml/NXopt_assembly.yaml

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* add_NX_defs_for_beam_path

* modifying_yaml_files

* fixing_nyaml_make_file

* Adjusted files with Sandor together according to
earlier hardcoded .nxs file

* Added the missing nxdl.xml files via nyaml2nxdl
Version=0.0.8 was used for nyaml.

* moved created nxdl.xml files to correct directory

* Suggestions to fix ci/cd by in NXtransfer_matrix_table.yaml

Co-authored-by: Florian Dobener <[email protected]>

* renaming transfer_matrix_table to beam_transfermatrix_table and opt_element to beam_device; also merging NXopt_beam to NXbeam

* remove old nxdl files

---------

Co-authored-by: Ron Hildebrandt <[email protected]>
Co-authored-by: Lukas Pielsticker <[email protected]>
Co-authored-by: Florian Dobener <[email protected]>
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
lukaspie pushed a commit that referenced this pull request Sep 24, 2024
* Add nexus definitions/files for beam path description

* Update base_classes/nyaml/NXopt_assembly.yaml

Co-authored-by: Lukas Pielsticker <[email protected]>

* Update base_classes/nyaml/NXopt_assembly.yaml

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* add_NX_defs_for_beam_path

* modifying_yaml_files

* fixing_nyaml_make_file

* Adjusted files with Sandor together according to
earlier hardcoded .nxs file

* Added the missing nxdl.xml files via nyaml2nxdl
Version=0.0.8 was used for nyaml.

* moved created nxdl.xml files to correct directory

* Suggestions to fix ci/cd by in NXtransfer_matrix_table.yaml

Co-authored-by: Florian Dobener <[email protected]>

* renaming transfer_matrix_table to beam_transfermatrix_table and opt_element to beam_device; also merging NXopt_beam to NXbeam

* remove old nxdl files

---------

Co-authored-by: Ron Hildebrandt <[email protected]>
Co-authored-by: Lukas Pielsticker <[email protected]>
Co-authored-by: Florian Dobener <[email protected]>
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
lukaspie pushed a commit that referenced this pull request Oct 16, 2024
* Add nexus definitions/files for beam path description

* Update base_classes/nyaml/NXopt_assembly.yaml

Co-authored-by: Lukas Pielsticker <[email protected]>

* Update base_classes/nyaml/NXopt_assembly.yaml

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* Apply suggestions from code review

Co-authored-by: Lukas Pielsticker <[email protected]>

* add_NX_defs_for_beam_path

* modifying_yaml_files

* fixing_nyaml_make_file

* Adjusted files with Sandor together according to
earlier hardcoded .nxs file

* Added the missing nxdl.xml files via nyaml2nxdl
Version=0.0.8 was used for nyaml.

* moved created nxdl.xml files to correct directory

* Suggestions to fix ci/cd by in NXtransfer_matrix_table.yaml

Co-authored-by: Florian Dobener <[email protected]>

* renaming transfer_matrix_table to beam_transfermatrix_table and opt_element to beam_device; also merging NXopt_beam to NXbeam

* remove old nxdl files

---------

Co-authored-by: Ron Hildebrandt <[email protected]>
Co-authored-by: Lukas Pielsticker <[email protected]>
Co-authored-by: Florian Dobener <[email protected]>
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants