Fairmat 2024: proposal on photoemission spectroscopy (MPES) #1424
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Hi folks, as this vote ticks along, note that @lukaspie found a couple issues in this PR, which he has proposed addressing through FAIRmat-NFDI#382, which would be merged into this one (#1424). I don't believe anything there affects this vote, so I'm happy with merging it in, but I'll wait a day or so to see if there's any feedback. Reasonable? This seems a nice compromise between needing to start the vote over and with not moving too fast. Thanks! |
Contributor
Author
|
Remaining open conversations:
|
Contributor
|
Vote has passed! Once the reviews have finished, comments have been resolved, an approval posted, this can be merged. |
lukaspie
force-pushed
the
fairmat-2024-mpes
branch
from
3f59ef9 to
0cf0ec7
Compare
Contributor
Author
|
@PeterC-DLS is there anything blocking us from merging this PR? All comments have been addressed and CI/CD is passing. |
Contributor
Author
@PeterC-DLS thanks for the thumbs-up, can you then also please approve the PR? I think it needs at least one formal approval before I can merge it. |
Contributor
Author
|
@PeterC-DLS I only realized after merging that this branch had 1281 commits on top of main, which are now all part of the repo history. While these are all valid commits (they are basically all from the work that we have been doing over at https://github.com/FAIRmat-NFDI/nexus_definitions), I am not so sure that this was the best way to go. Is there any standard practice within this repo what one should do when merging? Should commits from a PR be squashed? If having these large number of PRs in the history is problematic, maybe it would be good to revert the merging of this PR and then bring the same contents again (this time using a squash commit). On the other hand, if a squash commit is applied, we loose the contributions from everybody who worked on these branches, which might also not be fair. We will have the same problems for #1422, #1423, and #1425 by the way. |
This was referenced Jun 4, 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 <florian.dobener@physik.hu-berlin.de> Co-authored-by: Ron Hildebrandt <RonHildebrandt@uni-leipzig.de> Co-authored-by: mkuehbach <markus.kuehbach@physik.hu-berlin.de> Co-authored-by: Ron <139139971+RonHildebrandt@users.noreply.github.com> Co-authored-by: Markus Kühbach <mkuehbach@users.noreply.github.com> Co-authored-by: rettigl <l.rettig@web.de> Co-authored-by: RubelMozumder <32923026+RubelMozumder@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This brings in the application definition
NXmpestogether with its specializationNXmpes_arpesandNXxpsand related new base classes.NXmpesis designed to be the "umbrella" application definition for multi-dimensional photoemission spectroscopy (this is what we call MPES).NXmpeshas been incubating in the contributed definitions for more than xx years now and has been heavily tested within the communits. We (as FAIRmat) also organized two workshops with technology partners (i.e., companies building photoemission spectroscopy setups + scientists from the community) to come to a community agreement. Thus, we propose to add it to the NeXus standard as an application definition.On top of
NXmpes, two specializations have been added:NXmpes_arpes(for ARPES data) andNXxps(for XPS data). These are meant to specialize/clarify the concepts inNXmpesfor the specific scientific domain. In the case of XPS, the terminology closes follows that of the existing ISO standard for vocabulary in surface chemical analysis and, if applicable, makes direct links where terms are used in the same way. It also uses the community-agreed coordinate system (see ISO standard for data transfer in surface chemical analysis and #1415).Alongside these new application definitions, there come several new base classes:
NXelectronanalyser: for describing all components of a typical electron analyser used in such experimentsNXcollectioncolumn: for describing the lens system of a photoemission spectroscopy setupNXenergydispersion,NXspindispersion: for describing the energy/spin dispersive part of the electronanalyserNXmanipulator: for describing the multi-axes manipulator that typically holds the sample as well as any sensors/acutators mounted on it.NXdeflector(moved from contributed): for describing deflectors used in electron analysers.Most of these were already previously part of the contributed definitions.
NXfit,NXpeak,NXfit_function: these are all used for describing a fit to a dataset. In XPS specifically, they are used to define a peak model (consisting of multiple fits to the data). They are meant as a rigorous description of the procedures, data models, and constraint used in such fitting operations. Note that here, we don't use any formulas, but ratherformula_description, in line with what was discussed in the NIAC meeting 2024 (see discussion here: Math support in NeXus #711 (comment)).In addition to these base classes that are specificly discussed above, this PR contains many more additional new base classes and changes to existing base classes. Most of these have been discussed already, but the PRs are not yet merged. Note that these changes have already been brought to this branch right here to make the CI/CD pass (as several new base classes also depend on each other). Eventually, it is expected that oncee these PRs are finalized and this branch has been rebased, this PR here will be much smaller. Specifically, this PR depends on
Fairmat 2024: add NXpositioner, shape, and size to NXaperture #1409Fairmat 2024: several new base classes in NXsample and NXsample_component #1413Fairmat 2024: actuators and sensors in NXenvironment #1414FAIRmat 2024: additional base classes in NXinstrument #1419->mergedFairmat 2024: specialized base classes on top of NXprocess #1420->mergedFor a full list of (inter)dependencies, see discussion here: #1464 (comment).