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

NXxas refactoring #1352

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

NXxas refactoring #1352

wants to merge 11 commits into from

Conversation

woutdenolf
Copy link
Contributor

@woutdenolf woutdenolf commented Feb 10, 2024

WIP: everything will be moved to contributed_definitions once this is up for review.

Contributing: PR to the main branch of https://github.com/XraySpectroscopy/nexus_definitions (merge commits will be removed regularly).

Rendering: https://hdf5.gitlab-pages.esrf.fr/nexus/nxxas/classes/applications/NXxas_new.html

NXDL questions:

- how to re-use an enum
- when enum field X has a specific value, make field Y required OR how to define enum properties
- possibly link to
- how to make a field required in a base class

@lukaspie
Copy link
Contributor

Hi @woutdenolf, I came across this PR by chance and noticed that you are working on a base class for describing an electron level. In the context of photoemission spectroscopy, we in FAIRmat have also been working on something very similar: https://github.com/FAIRmat-NFDI/nexus_definitions/blob/fairmat/contributed_definitions/NXelectron_level.nxdl.xml. In fact, some of the base class we developed was originally adapated from another XAS-related PR: #1293. Would you be open to harmonizing the concept that we were developing with your approach here? I think the two approaches are already very similar right now.

@woutdenolf
Copy link
Contributor Author

@lukaspie Yes since we are doing the same thing we should try to converge. We are currently discussing this very topic: XraySpectroscopy#2. Feel free to get involved.

The XAS working group will work in https://github.com/XraySpectroscopy/nexus_definitions. It would be helpful to add comments in that repo. This PR will be a draft and is subject to lots of changes until we are finished.

@lukaspie
Copy link
Contributor

@woutdenolf great, thanks for the pointers. I will add some comments there.

@woutdenolf
Copy link
Contributor Author

another XAS-related PR: #1293

This was the first attempt before we created the XAS working group. It will most likely be closed.

@whs92
Copy link

whs92 commented Mar 13, 2024

@woutdenolf after asking a question to the nexus mailing list and reading:

#1011
#1293

I arrived here :)

I have had a look at the proposal you made to the NXxas_new

I am particularly interested in the topic raised in #1011 of adding the ability to work with multiple detectors. I see that you've added the entry intensity that is still tied to a single mode (TFY, PFY, TEY etc). How might I add other signals acquired at the same time? How might I add more than one PFY signal?

Do you plan to include the suggestions from @padraic-shafer in https://github.com/padraic-shafer/definitions/blob/1011-multixas/contributed_definitions/NXmultixas.nxdl.xml

When is the next planned meeting of the working group? Can I come along?

@woutdenolf
Copy link
Contributor Author

Currently we only support a single XAS spectrum. I'm very much open to support multi-detector and/or multi-scan. You are very welcome to join the XAS working group. I'll contact you directly.

<field name="name"/>
<field name="probe">
<enumeration>
<item value="x-ray"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to allow the "electron energy loss" mode, then you should also allow an "electron" probe type.

</enumeration>
</field>
<group type="NXelement" name="element">
<doc>Excited element</doc>
Copy link
Contributor

Choose a reason for hiding this comment

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

What if I want to do a scan that includes multiple absorption edges (e.g. Carbon K-edge and K L-edge)? Can I specify multiple elements?

</item>
<item value="electron yield">
<doc>
TODO total or partial electron yield
Copy link
Contributor

@benajamin benajamin Sep 27, 2024

Choose a reason for hiding this comment

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

Does this include Auger electron yield? Can we include a way to specify the observed electron energy range? Or should that be a detail for NXdetector?

</dimensions>
</field>
</group>
<group type="NXdetector" name="i0" optional="true">
Copy link
Contributor

@benajamin benajamin Sep 27, 2024

Choose a reason for hiding this comment

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

The i0 should have a corresponding NXmonitor group. There are also many ways of measuring an i0 that is useful to document in the file. For example, is the i0 measured in parallel with the sample absorption (or its proxy), or is it measured in sequence (e.g. before or after the sample), or is it sequential, but interleaved?

There is also question of how "clean" the i0 data is, as in how directly/accurately does the i0 measurement follow the ideal incident flux signal? This issue is particularly significant for C K-edge measurements - one can read my article on the topic to further understand this issue.

@ounsy
Copy link

ounsy commented Sep 29, 2024

#1352 (comment)
@whs92
@woutdenolf
I also got a request for that from one of our beamlines at SOLEIL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

6 participants