Skip to content

Conversation

@lukaspie
Copy link
Contributor

@lukaspie lukaspie commented Sep 24, 2024

Refactored version for NXapm including feedback from the atom probe microscopy/field-ion microscopy community.

This PR depends heavily on others that make changes to existing base classes or introduce base classes that are to be used outside of APM as well. Thus, it will be much smaller once the discussion on these PRs is done and this particular branch here has been rebased:

#1415
#1407
#1408
#1412
#1413
#1414
#1419
#1420
#1486

There are however several new base classes that are currently only used within NXapm that are introduced here:

  • NXapm_charge_state_analysis
  • NXapm_ranging
  • NXapm_reconstruction
  • NXchemical_composition
  • NXcs_* base classes for documenting computational aspects and filtering
  • NXevent_data_apm
  • NXinstrument_apm

Aim and design of the appdef is as follows:
NXapm envisaged to be used for documenting measurement and computer simulation of field evaporation experiment.
Instrument part split in two parts:

  • ENTRY.measurement.instrument, for all those metadata that do not change when event data are collected
  • ENTRY.measurement.events.EVENT_DATA_APM.instrument for all those metadata that do change during events
    This idea allows to not always repeat like with which instrument and hardware components the events were collected

In field evaporation experiment one measures:

  • ToF, timing data on at least two delay-line detectors to get ion impact positions
  • From this one computes, ions, their chemical and/or nuclides, ultimately resulting in a model of the evaporated specimen the so-called reconstruction. The reconstruction is a point cloud, each point can have an ion label.
  • Ion labels are computed based on peak finding and peak identification in calibrated mass-to-charge-state ratio histograms, using methods borrowed from other ToF mass spectrometry and other spectroscopy techniques
  • All these processing steps of data are modelled as inheriting from NXprocess

Contrary to the previous version of this proposal:

  • We have reduced further the number of new base classes and tried to consolidate them even further.
  • We have added references to proprietary concepts of AMETEK/Cameca where known and reliable these are these CRunHeader short docstrings
  • We backup this example with a metastudy where we have collected the majority of datasets that are available online and use NXapm to map the content of these studies into NeXus/HDF5 files via www.github.com/FAIRmat-NFDI/pynxtools-apm

Points of discussion from my side @mkuehbach:

  • NXchamber should be replaced by NXcomponent
  • NXion should be replaced by NXatom
  • NXevent_data_apm could be replaced by NXevent_data but for this last point to happen at least NXevent_data needs to be refactored as currently its scope is limited to neutron science, specifically " NXevent_data is a special group for storing data from neutron
    detectors in event mode."

Note that in FAIRmat we have a comprehensive set of appdefs and classes built around NXapm that we currently would like to keep and rather evolve only in contributed_definitions, see related PR:

  • Make PR for their evolution as to how we currently use them in FAIRmat to provide examples for the community

Examples for usage of computational geometry was explored especially in APM with the relevant separate geometry-focused PRs #1421 #1532

atomprobe-tc and others added 30 commits August 14, 2024 10:22
Update optical spec website to include optical spectroscopy and raman
Add edge cases to namefit documentation
- Angle between beam and sample normal-> needed for determination of probing depth
- Angle between beam and analyzer axes -> needed for data analysis
Instead of having two fields on the instrument for the angle between beam and sample and the angle between beam and analyser, this information is stored in depends_on and NXtransformations in NXinstrument and NXsample, respectively.
 - The transmission function contains an additional NXnote field "file" describing the file in which the TF data is stored.
- Angle between beam and sample normal-> needed for determination of probing depth
- Angle between beam and analyzer axes -> needed for data analysis
@mkuehbach
Copy link
Contributor

As I mentioned in the telco, the definition of NXprocess is ambiguous with its text:

Document an event of data processing, reconstruction, or analysis for this data

as this data does not specify what or where this data is!

Also in elsewhere in the manual:

Data reduction and analysis programs are encouraged to store their results in NeXus data files. As far as the necessary, the normal NeXus hierarchy is to be implemented. In addition, processed data files must contain a NXprocess group. This group, that documents and preserves data provenance, contains the name of the data processing program and the parameters used to run this program in order to achieve the results stored in this entry. Multiple processing steps must have a separate entry each.

So it would be good to clarify how one or more NXprocesses should be used i.e. where the results are stored and how.

8780d90 and the commit that follows fixes the issue with this data, I agree that the docstring of sequence_index is unclear in the sense that what happens when NXprocess instances are nested, does the numbering include all instances in the appdef or only the siblings of the group in which an instance of NXprocess is located. This is a question of how to index workflow step when the appdef is used to serialize a chunk of or the all steps of an eventually complex workflow, i.e. a workflow graph.

I am not convinced that this should block this PR though.

@mkuehbach
Copy link
Contributor

@benajamin done with all points that remaining, conversations all resolved, latest version from @lukaspie (mpes) included, please accept, @phyy-nx this can be moved to a vote, as was decided at the telco yesterday, fyi @sanbrock @lukaspie

@rayosborn
Copy link
Contributor

Since this also changes the NXprocess base class, I would like to merge #1559 first. @phyy-nx, could you please confirm that's OK.

@mkuehbach
Copy link
Contributor

mkuehbach commented Jun 16, 2025

telco now: bring #1559 and this PR to a vote @rayosborn @PeterC-DLS agree,
two blockers to solve first:
@rayosborn writes the paragraph
@PeterC-DLS will review #1559

mkuehbach pushed a commit to FAIRmat-NFDI/nexus_definitions that referenced this pull request Jun 17, 2025
…but which is equally useful for this nexusformat#1423 on EM cuz either the base class are anyways envisioned to be used from APM as is or these were good suggestions
@mkuehbach
Copy link
Contributor

mkuehbach commented Jun 18, 2025

#1559 was reviewed and approved @phyy-nx voting can start as discussed at the last telco

@phyy-nx
Copy link
Contributor

phyy-nx commented Jun 18, 2025

Hi folks, as discussed on the Telco, this PR can now me moved to an online vote. Please vote using emojis (👍 for yes, 👎 for now, anything else e.g. 👀 for abstain). Voting will close in 2 weeks.

Note, this PR is dependent on #1559.

Thanks!

@rayosborn
Copy link
Contributor

rayosborn commented Jun 23, 2025

I'm afraid that I don't have the bandwidth to give it the detailed scrutiny this PR requires. I know this has been debated at a number of Telcos, but the sheer volume of changes (48 files) makes it extremely difficult to keep track of them. I have a couple of comments.

First, I think it would be helpful in the future if we separate such PRs into domain-specific classes, which have little impact outside their domain, and more general base classes, so that those working outside the domain are able to more easily see how it might impact their own work. Personally, I am willing to give much greater deference to APM specialists making their own decisions about NXapm_ classes than classes I am likely to use.

Second, I think it is now urgent to develop methods of separating base classes into their respective domains. In NeXpy, we have pull-down menus that allow users to select the class of a new group based on their context. These menus are about to get ~20% longer. I think I need a mechanism for creating domain-specific submenus. I could search for 'NXapm' or '_apm' (I think both appear in this PR) but it would be better to add a file-level attribute specifying if the base class is domain-specific or more general.

@mkuehbach
Copy link
Contributor

mkuehbach commented Jun 23, 2025

Thank you @paulmillar for your comments as these were non-blocking edits on docstrings exclusively adding clarification and rephrasing, I've implemented them, they do not change the meaning of the concepts.

@phyy-nx
Copy link
Contributor

phyy-nx commented Jul 6, 2025

The vote has passed, thank you. @rayosborn it would be great to discuss your concerns in tomorrow's telco

@mkuehbach mkuehbach merged commit ecc9361 into nexusformat:main Jul 13, 2025
2 checks passed
@PeterC-DLS PeterC-DLS added this to the NXDL 2025 milestone Aug 11, 2025
sanbrock pushed a commit that referenced this pull request Aug 25, 2025
* Removed some unneeded comments

* Add description of instrument orientation

- Angle between beam and sample normal-> needed for determination of probing depth
- Angle between beam and analyzer axes -> needed for data analysis

* Move orientation of instrument parts

Instead of having two fields on the instrument for the angle between beam and sample and the angle between beam and analyser, this information is stored in depends_on and NXtransformations in NXinstrument and NXsample, respectively.

* Update transmission function information

 - The transmission function contains an additional NXnote field "file" describing the file in which the TF data is stored.

* Update documentation, add comments for discussion

* fix NXmpes yaml

* update to current NXmpes status

* refine NXtransformations

* add coordinate system and coordinate_system_set

* Adds mpes_xps to mpes structure

* rename to NXxps

* initial attempt at modeling XPS peak fitting

* add lineshape to NXpeak

* update lineshape in NXpeak

* add NXbackground to NXfit_region

* add fitting method to NXpeak_model

* remove NXtransformations from NXcoordinate_system

* restructure NXfit with background and peaks

* change to NXcoordinate_system_set, use NXfit in NXxps

* add fit_function and parameters

* use NXfit_function for peaks and backgrounds

* use NXdata in NXfit

* use new peak fitting in NXxps

* add missing class name in NXxps

* remove underscores from variable names in NXfit

* update nyamls

* switch axes docs in NXfit

* use old nyaml dim notation for now

* fix math notation

* make nxdls

* regenerate with nyaml==0.0.8

* make NXfit, NXpeak multi-dimensional

* redefine XPS coordinate system, small changes to XPS peak fitting

* required energy axis in XPS

* fix dimensions issues

* regenerate NXxps

* make energy an NX_NUMBER

* remove unneeded depends_on enumerations

* align symbols across all fit-related definitions

* remove single and repeated parameters in fit

* fix dimensions in NXpeak and NXfit_background

* use globlal_fit and error_function in NXfit, fix dimensions

* docstring fixes in NXfit

* NXfit extends NXprocess

* fix math notation in NXfit_function

* fix link in NXfit_function

* change requiredness for formulas in XPS fit

* small fixes for links

* fix issues with manual build

* fix math notation

* more math fixes

* regenerate NXxps and NXpeak nxdl files

* fix rebasing issues

* make NXpeak spectrum-independent

* fix rebase conflicts

* fix nyaml-nxdl inconsistency

* add notes from both workshops

* added make nxdl and make local

* lowercase enumeration, source_type todo added, doc for ellipsometry_type

* HOW LONG? does it take to learn to make nxdl and local before commiting >.<

* Fixes in EM

* Merging NXem_adf into NXem_img, enabling microstructure reconstructions as dependants of specific method results, introduction of NXatom_set to discuss generalization of NXion towards enabling descriptions of groups of atoms, also to lias with FAIRmat Area C

* Removal of non-standardized assumption that i, j, k can be used as counter variables in case one does not know the actual dimensions but one is sure it is not a scalar

* add missing elements in ellipsometry from specialized NXlens_opt

* Add todo notes for beamsize and shape  description

* typo and note for possible rework/reconsideration of NXfabrication

* Suggestions from lukaspie

* Fixes apm

* Added undefined enum in NXidentifier

* use NXidentifier in NXsample and NXfabrication

* rename sample name in NXoptical spectroscopy

* make ellipsometer_type optional

* NXidentifier in NXoptical_spectroscopy instrument

* remove NXsample name docs from NXoptical_spectroscopy

* add serial_number to NXfabrication

* use NXidentifier in NXmpes

* use NXidentifier consistently across all our contributed definitions

* decode binary strings in nxdl_utils.py

* add function for decoding

* use NXidentifier in NXentry

* small changes to NXbeam and NXmonochromator

* typo fixes

* proper string decoding

* remove unneeded imports

* black formatting

* remove code and move to pynxtools

* remove unneeded requirements

* escape black check

* ignore flake8 issue

* isort imports

* use less strict version of str decoding

* slightly enhanced version of str decoding, test

* remove flake error catching

* reset requirements.txt

* reformatting

* remove strict type checking

* add support for list of bytes

* formatting

* add pulse_delay to NXbeam

* recreate nyaml file

* regenerate nyaml files

* various small fixes are merging main and fairmat

* fix string No in NXelectron_level

* remove unused MANIFEST.in

* manually reset NXem yaml file

* format base classes and applications in accordance to existing NIAC version

* revert changes to NXtransformations docs

* add CSS tweak to hide summary

* add css file

* Suggestions from Lukas

* fix dev_tools tests
black code style

* remove use of details_summary_hide

* change refs in NXarpes

* move doc bundles to applications and base_classes

* Reverting the proposal that NXmicrostructure is not just proposed for contributed but implicitly as a dependency of NXem, NXem_ebsd, NXem_img proposed for voting and acceptance

* Revert one more microstructure

* move new definitions to application and base_classes

* small update to mpes-structure

* update category for NXxrd.nxdl. (#296)

* remove fairmat-specific files

* remove nyaml files

* move NXdata_mpes* classes back to contributed

* revert small changes to the dev_tools

black formatting

isort dev_tools

* change docs in NXcircuit

* pull out modifications for fairmat-2024-em

* remove classes from contributed that have been moved to base_classes

* update type in NXsource

* add test on NXem application definition

* use pathlib.Path

* parametrize test cases with fixtures

* resolve merge conflict on NXdetector

* black formatting on tests

* remove double NXpositioner from NXaperture

* NXem_method removed as retrospectively considered a too tiny modification of NXprocess

* Edits on em-specific base classes

* Edits on NXem_ebsd based on Autumn NIAC feedback

* Latest edits on NXem_correlation before I propose to remove it cuz retrospectively I feel that this base class is too focused on EBSD surplus alone has an insufficient amount of concepts to warrant standardization right now, I like that there is a base class which allows to group signals of different (computed/or measured origin) that is standardized on the same physical or simulated material volume, however, this could also be achieved via specializing an NXimage in an application definition and augmenting it by NXprocess-relevant fields

* Retracting NXem_correlation for now

* Retraction of NXevent_data_em_set for the reason that just an additional grouping in an appdef can also be achieved by just adding event_data_set(NXobject) and having current NXevent_data_em_set docstring there inside the appdef

* Edits in preparation for moving NXinteraction_volume_em to the application definition NXem, the top-level docstring and scientific context why having such a class is useful but the class has not been worked out in detail enough to warrant having an own base class, it should just be moved to NXem/entryID/em_sim also strengthening the point that NXem can be used for computer simulations of EM matters.

* Renaming of core base classes NXimage, NXspectrum fix incorrect usage of reserved NeXus suffix _set

* Refactoring of NXcrystal_structure to NXphase

* Renaming of NXcrystal_structure and NXphase complementing a refactoring of the fundamental matter based description of the FAIRmat proposal like changes to rename NXion to NXatom and possibly NXmicrostructure to NXsystem

* Edits on NXphase formerly NXcrystal_structure

* Renamed NXion to NXatom reintroduction of NXcrystal_structure to use it as a group in NXphase

* Fixing NXcrystal_structure

* Completed refactoring and editing of NXatom, NXphase, NXcrystal_structure, groups. To increase the possibility for acceptance, the atom-probe-specific aspects of NXion now NXatom have been removed and will be reintroduced via NXapm_ion as a specialization of NXatom for usage in the field of atom probe

* Edited NXebeam_column

* Edits on NXevent_data_em

* Edited NXibeam, moving on to NXimage

* add specified coordinate systems for EM/APM

* Working on image and spectrum

* Unify concept naming convention from *_identifier to identifierNAME

* Refactoring and removal of NXcrystal_structure in favour for NXunit_cell

* Further edits and refactoring

* Renamed NXrotation_set to NXrotation

* Further working on em-specific base classes, next up: i) NXscanbox_em, ii) NXstage_lab consolidation with NXmanipulator, and NXem appdef

* Completing first round of edits on base classes for NXem

* Removing NXstage_lab after discussion with @lukaspie as a result of which NXstage_lab will be replaced in NXem and NXapm appdefs via NXmanipulator from #1424 augmented by remaining concepts and contextualizing top-level doc string from NXstage_lab pulled into the appdef. As a side effect thereby also docstring become more focused on the relevant techniques and a base class is not talking only with examples from atom probe

* Removed NX*_reference_frame.nxdl.xml after having spent more time with negotiation and coming to the conclusion that maybe indeed these base classes are tiny to warrant becoming part of the standard, instead NXcoordinate_system should be specialized in respective appdefs but the names sample_reference_frame, processing_reference_frame, and detector_reference_frame are good candidates

* Edits in utility base classes of NXem

* Removal of classes in contributed definitions that had been used while designing NXem iteratively and are now superseeded by better alternatives, NXaberration, NXcircuit, NXem_ebsd, NXimage, NXspectrum, editing of the EM-specific part of the documentation to document these changes

* Editing and reviewing method-specific classes

* BREAKING CHANGE NXem_msr to NXinstrument_em following the design of NXapm, such that the same modular group of classes NXinstrument_em can be used in the NXem appdefs in both ENTRY.measurement.instrument, ENTRY.simulation.instrument, ENTRY.measurement.events, ENTRY.simulation.events

* Refactored NXem, last steps remaining, i) check nameType partial, ii) stage

* Fixed templated concept paths in the tests

* Added NXmanipulator from the fairmat-2024-mpes branch (commit 3255716) to refactor NXstage_lab based on

* Remaining issues solved, NXstage_lab refactored using NXmanipulator

* Remove shell script for processing specific yaml to nxdl

* Refactoring work on base classes used in EM, here specifically with a focus on changing NXobject to NXcomponent

* Fixed use of incorrect syntax for open enumerations

* EM ready for review

* Reverting the automatically and incorrectly from github created NXcg_* classes in base_classes, this PR does not touch computational geometry base classes!

* Reversion

* Reversion 2

* Reversion 3 contributed

* Slight edits to cure effects that reversions had on compileability of the definitions

* Reverting NXevent_data

* Revert NXhistory

* Synced back with the latest edits that we have done in FAIRmat including several fixes of nameTypes and relaxing constraints on what previously where partial nameType concepts with the allcaps suffix ID but which turned out to be eventually not effective enough for the time being so we when for the more classical allcaps nameType any, examples are userID refactored to USER, programID to PROGRAM, often using the implicit nameType any. We consider this branch now feature complete for revision

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - aberration, atom, chemical_composition

* Move NXcoordinate_system_set back to contributed as it will overall not proposed anymore but is still required by NXapm* but this will be dealt with in the apm specific PR, so for sure remove it from the EM proposal

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - corrector_cs, computer cs classes

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - deflector, ebeam_column

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - em-method-specific

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - new NXbag-style tiny base classes em_measurement, em_simulation, event_data_em, ibeam_column, image

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - instrument_em, NXbag-style interaction_volume_em, promoting ion to base class

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - lens_em, manipulator

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - optical_system_em, phase, peak, program, pump

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - rotations, scanbox_em, spectrum, unit_cell

* Use prepared classes of with the latest changes for the em proposal from FAIRmat/nexus_definitions ca0c142 - new NXbag-style tiny base class NXroi, proposal adding NXcircuit mentioned in nexusformat/definitions PR1534

* Reviewer version complete only potential CI/CD issues and rst documentation remains in need for fixing

* Obvious fixes for NXem tests

* More fixes on these em tests

* Linting

* Fixed inconsistence with use of reserved keyword variable_errors

* Changes on manual/source that were left due

* Using newest CI/CD workflows from the FAIRmat branch to make a mock PR for rendering an HTML page that the reviewers for the NIAC Autumn 2024 proposal can review

* Enable manual triggering

* Adding gas_injector to complete all relevant components for a FIB/SEM, reorganized scan_controller to better allow separating storage of the controller scanning the ebeam and the ibeam, making also cleaner the design of where to report on blankers as specific types of deflectors

* Two-page introduction to give reviewers with a top-level view on NXem

* Completed two-pager on NXem, awaiting discussion with the reviewers

* cspell activated

* Proof-reading two-pager.

* Some edits of NXem that were motivated by the previous two niac telco's as well as the suggestions that @benajamin made on NXapm #1422

* Revert cspell and changes on CI/CD

* Replace single quote with double quote in XML header

* Added feedback that was made mainly by @benajamin on #1422 but which is equally useful for this #1423 on EM cuz either the base class are anyways envisioned to be used from APM as is or these were good suggestions

* Fix broken reference to renamed NXroi in apm

* Adding figures as svg and draft for captions for each figure, reporting each of the three main types of instruments covered SEM, SEM/FIB, (S)TEM

* Add suggestions from members of C. Koch's group, finished method-specific introductory guide to the classes, finished drafting the captions for the figures for SEM, FIB, (S)TEM

* Fix the CI pipeline

* Reintroduced -W option for makefile

* Try to fix that with the current CI and sphinx settings, using directly svg vector graphics are unsupported when compiling the latex part of the NeXus documentation

* Fix that imagemagick likely no longer uses its less robust internal svg rendered but the one from inkscape to deal with svg vector graphics

* Fix missing whitespace

* Comments from Fernan Saiz and Jordi Arbiol

* A few more quality fixes on the EM proposal

* Adding correct test_nxdl_utils from main

* Reintroduce NXcoordinate_system_set from upstream/main to make NXapm_ contributed rendering

* Refactoring of names for NXinteraction_volume_em, NXlens_em, NXoptical_system_em, NXscanbox_em

* Removed unexpectedly placed character that caused the CI to break

* Addressing @phyy-nx's comment on use of NX_WAVENUMBER in NXimage

---------

Co-authored-by: domna <[email protected]>
Co-authored-by: Ron Hildebrandt <[email protected]>
Co-authored-by: mkuehbach <[email protected]>
Co-authored-by: Ron <[email protected]>
Co-authored-by: Markus Kühbach <[email protected]>
Co-authored-by: rettigl <[email protected]>
Co-authored-by: RubelMozumder <[email protected]>
@mkuehbach mkuehbach deleted the fairmat-2024-apm branch September 17, 2025 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting input NIAC should review The NIAC should review/discuss

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NXapm