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.

Sign up for GitHub

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

Jump to bottom

Fairmat 2024: additions and clarifications in NXbeam #1408

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

Fairmat 2024: additions and clarifications in NXbeam #1408

wants to merge 20 commits into from

Conversation

lukaspie
Copy link
Contributor

No description provided.

prjemian
prjemian reviewed Sep 23, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml Outdated
@@ -61,11 +61,45 @@
<doc>Distance from sample. Note, it is recommended to use NXtransformations instead.</doc>
</field>
<field name="incident_energy" type="NX_FLOAT" units="NX_ENERGY">
<doc>Energy carried by each particle of the beam on entering the beamline component</doc>
<doc>
Energy carried by each particle of the beam on entering the beamline component.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Energy carried by each particle of the beam on entering the beamline component.
Energy carried by each particle of the beam on entering the beamline location.

NXbeam defines the properties of the beam at a specific location.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, has been changed.

@sanbrock sanbrock mentioned this pull request Sep 26, 2024
Open
1 task
PeterC-DLS
PeterC-DLS reviewed Sep 29, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml Outdated
</field>
<field name="frog_trace" type="NX_FLOAT">
<doc>
FROG trace of the pulse.
Copy link
Contributor

Choose a reason for hiding this comment

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

Spell out what FROG stands for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

PeterC-DLS
PeterC-DLS reviewed Sep 29, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml
Group delay dispersion of the pulse for linear chirp
</doc>
</field>
<field name="previous_device">
Copy link
Contributor

Choose a reason for hiding this comment

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

The general way is place an NXbeam in a device in NXinstrument - whether this is good enough or a new way to chain devices together should be a NIAC decision.

PeterC-DLS
PeterC-DLS reviewed Sep 29, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml Outdated

In the case of a monochromatic beam this is the scalar energy.
Several other use cases are permitted, depending on the
presence of other incident_energy_X fields.
Copy link
Contributor

Choose a reason for hiding this comment

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

Add backticks to emphasize field names to RST formatting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed, thanks

@lukaspie lukaspie linked an issue Sep 29, 2024 that may be closed by this pull request
NXbeam #1447
Open
1 task
phyy-nx
phyy-nx reviewed Sep 29, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml Outdated
Example: for lightsource brilliance (SI) 1/(s.mm2.mrad2).
Here: SI units are V2/m2.
</doc>
</attribute>
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this attribute block, and change units for incident_polarization to V^2/m^2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

phyy-nx
phyy-nx reviewed Sep 29, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml Outdated
Example: for lightsource brilliance (SI) 1/(s.mm2.mrad2).
Here: SI units are V2/m2.
</doc>
</attribute>
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this attribute block, and change units for final_polarization to V^2/m^2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

phyy-nx
phyy-nx reviewed Sep 29, 2024
View reviewed changes
base_classes/NXbeam.nxdl.xml Outdated
<doc>
Here: SI units are 'J/m2', customary 'mJ/cm2'.
</doc>
</attribute>
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete this attribute block, and change units for fluence to mJ/cm^2.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@prjemian
Copy link
Contributor

RST wraps computer code with double backticks:

* correct: ``energy``
* incorrect: `energy`

This is one of the most common typos in the documentation.

@lukaspie
Copy link
Contributor Author

RST wraps computer code with double backticks:

* correct: ``energy``
* incorrect: `energy`

This is one of the most common typos in the documentation.

You are right, I changed it here for our new contribution where we make use of backticks.

RubelMozumder
RubelMozumder reviewed Oct 9, 2024
View reviewed changes
Copy link
Contributor

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

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

I just randomly went through the file to be knowledged of the NAIC decision. I found some typos of bouble back tick while mentioning some fields.

base_classes/NXbeam.nxdl.xml Outdated Show resolved Hide resolved
base_classes/NXbeam.nxdl.xml Outdated Show resolved Hide resolved
base_classes/NXbeam.nxdl.xml Outdated Show resolved Hide resolved
domna and others added 16 commits October 16, 2024 10:33
@domna @lukaspie
Readds base classes for mpes (from commit 843283a)
80d7be9 
# Conflicts:
#	base_classes/NXaperture.nxdl.xml
@domna @lukaspie
Readds mpes relevant changes from 4b064d9
1a4ef9c
@domna @lukaspie
Making ci/cd happy again
a8eb979
@RubelMozumder @lukaspie
Regeneration of the nexus file for fixing the changes coming from old…
c47092c 
… version of yaml.

Removing unintensional comments
@RonHildebrandt @lukaspie
Add nexus definitions/files for beam path description (#183)
465010c 
* 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
remove changes to polarization in NXbeam
1735aaa 
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
@lukaspie
remove changes to final_polarization in NXbeam
b76028d 
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
@lukaspie
readd units to NXbeam polarization
ff7c0d7 
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
@lukaspie
small changes to NXbeam and NXmonochromator
a28e62e 
# Conflicts:
#	base_classes/NXmonochromator.nxdl.xml
#	base_classes/nyaml/NXbeam.yaml
#	base_classes/nyaml/NXmonochromator.yaml
@lukaspie
add pulse_delay to NXbeam
2824fb8 
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
@lukaspie
recreate nyaml file
fcca897 
# Conflicts:
#	base_classes/nyaml/NXbeam.yaml
@lukaspie
format base classes and applications in accordance to existing NIAC v…
1e82e79 
…ersion
@lukaspie
revert unintentional changes from cherry-pick
369bd61
@lukaspie
fix copyright in NXbeam
e7a5a58
@sanbrock @lukaspie
Update NXbeam.nxdl.xml
5b34d64 
suggestion from NIAC
@lukaspie
backticks to emphasize field names to RST formatting
c57ed41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RubelMozumder RubelMozumder RubelMozumder left review comments

@phyy-nx phyy-nx phyy-nx left review comments

@prjemian prjemian prjemian left review comments

@PeterC-DLS PeterC-DLS PeterC-DLS left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

NXbeam
8 participants
@lukaspie @prjemian @PeterC-DLS @phyy-nx @RubelMozumder @domna @RonHildebrandt @sanbrock