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

NXmx total_flux #986

Closed
lucagelisio opened this issue Feb 25, 2022 · 7 comments · Fixed by #1035
Closed

NXmx total_flux #986

lucagelisio opened this issue Feb 25, 2022 · 7 comments · Fixed by #1035
Assignees

Comments

@lucagelisio
Copy link

The quantity total_flux is currently defined as flux incident on beam plane in photons per second.
For pulsed light sources, like free electron lasers, the temporal profile of each pulse is rarely reliably known.
It would be therefore desirable and more accurate to express total_flux in photons.

Would it be possible to have either photons/second or photons as unit of measurement for total_flux?

@yayahjb
Copy link
Contributor

yayahjb commented Feb 25, 2022 via email

@lucagelisio
Copy link
Author

Thanks a lot Herbert, I like your proposal.
total_flux is a required entry, could this become recommended, or alternatively require this one or its integrated version?

@phyy-nx
Copy link
Contributor

phyy-nx commented Feb 25, 2022

Discussion from NIAC telco today: what we need is a way to specify that the user must supply one of total_flux, flux_integrated, or total_flux_integrated. There doesn't seem to be an easy way to write that specification using NXDL, so it's recommended this issue be brought up in the next NIAC meeting in early March 2022.

@phyy-nx phyy-nx added the NIAC should review The NIAC should review/discuss label Feb 25, 2022
@benajamin
Copy link
Contributor

I think that it could be accomplished by requiring a "pointer" field be present whose value is the name of the flux measure being provided. An existing example is the signal attribute for NXdata.

@phyy-nx
Copy link
Contributor

phyy-nx commented Mar 4, 2022

Feedback from Spring NIAC 2022: using an NXmonitor class may be better here. See NXstxm for example. (This comment is meant as a springboard for more discussion here).

@lucagelisio
Copy link
Author

This sounds sensible to me!

phyy-nx added a commit that referenced this issue Mar 15, 2022
- Add flux_integrated and total_flux_integrated, Useful where temporal profiles of flux are not known.
- Change total_flux to not be required
- Add NXbeam attribute flux, to point to which flux field is used
- Add optional NXmonitor for additional incident beam data

Closes #986
@benajamin
Copy link
Contributor

XML has a choice element that requires exactly one object from a set be present. It would need to be used in the NXDL schema to define a tag that you could then use in NXmx.

phyy-nx added a commit that referenced this issue Sep 15, 2022
Flux changes for NXmx

- Add flux_integrated and total_flux_integrated, Useful where temporal profiles of flux are not known.
- Change total_flux to not be required
- Add NXbeam attribute flux, to point to which flux field is used.   Includes a note that the user can provide a link to an NXmonitor with per-shot beam data

Closes #986

Co-authored-by: woutdenolf <woutdenolf@users.sf.net>
@benajamin benajamin removed the NIAC should review The NIAC should review/discuss label Sep 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants