-
Notifications
You must be signed in to change notification settings - Fork 66
Fairmat 2024: proposal on electron microscopy (EM) #1423
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
Conversation
- 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.
|
Telco, NIAC request to change names of some base classes:
|
I suggest to use NXelectromagnetic_lens, so that electrostatic lenses in XPS are also covered. |
|
Is complete, @phyy-nx voting can start |
|
Hi Markus, NX_WAVENUMBER as units for the diffraction axis makes sense in theory, but oftentimes the calibration isn't right or even known to convert the pixel coordinates of the diffraction detector to reciprocal space coordinates. Instead, we might want to store the diffraction data in pixels and rely on the NXdetector and NXbeam groups to calibrate and convert from pixels to reciprocal space coordinates. Is this allowed/feasible in NXem? Addressed via the last commit 31704f0 |
|
@phyy-nx another signature of support for NXem (found the paper again while cleaning up on my system): found here https://doi.org/10.1557/s43577-025-00882-2 |
|
Sweet find @mkuehbach! |
|
Hi all, this PR can now be moved to a vote. Please votes using emoji's, 👍 for yes, 👎 for no, anything else e.g. 👀 for abstain. Voting will close in 2 weeks! |
…D release (#400) * Fine-tuning of the naming conventions for identifier which is now a reserved NeXus keyword versus renaming many of these to index_offset, indices_* * Refactoring of identifier in favor for indices, harmonizing when to use NX_POSINT, NX_UINT, harmonizing that all index and indices use NX_INT * Recovered unplanned automated removal of docstring blockcomment whitespace * A few more whitespaces * phase_identifier * Update definitions for MPIE's compositionspace tool * Updates on NXmicrostructure, fine tuning of symbols, and concepts surplus proofreading * Fix CICD * Adding color_model to ipf, tsl default if mtex is not present, if mtex is present writing tsl and mtex * Reverting the use of NXobject in favor for new small base classes as per decision of the NIAC nexusformat#1423 (comment) * Further reverting for NXobject for APM-part of the proposal * Further removing of NXobject and indenting NXevent_data in NXem. After thinking further about grouping NXevent_data instance or not I thought it could also be a valid argument that as why should the standard force people to not have thousands of groups ending up at the same level of the hierarchy. Clearly, there is nothing which prevents sb from doing this for the example of HDF5 but performance-wise this is problematic, there are multiple examples even in commercial microscope software solutions where indeed HDF5 files with thousands and more groups at the same level are stored using HDF5, while there is overhead involved in this and searching by humans might be ineffective it is still a valid HDF5 file according to the HDF5 data model. Also NeXus with allowing an (NXentry) de facto allows to make an instance where thousands of e.g. entry1, entry2, ... are stored in the same HDF5 file also here no attempt has been made to build another group to just suggest strongly that people avoid this practice. Therefore, I removed one layer of indentation within NXem_measurement so that in an instance one now can have instrument, event1, event2, .. It is a design issue with NeXus that when we accept and wish that one cannot use NXobject as a plain structuring element but at the same time does not wish ones content to become at the schema level already non verified like when using an NXcollection there is no practice to group content other than making a new base class such for the sake of it holding the grouping. * Completed reverting the usage of NXobject in places other than than root of an app or class def * Reverting all cases where patterns where used like concept is of nameType: any with docstring Instances should by nameType partial and prefixID, lot 1, apm and cg * Removal of obsolete NXcoordinate_system_set to reflect how the discussion with the NIAC when specifically as documented in https://github.com/nexusformat/definitions/pull/1415/files * Reverting nameType any to partial pattern, lot 2 microstructures * Reverting nameType any to partial pattern, last lot NXem * Addressed phyy-nxx comment on using American spelling throughout * Addressed and worked in phyy-nx review comments from nexusformat#1423 starting with the inline comments, next step, address remaining comments that were made directly in the PR discussion * Work on the EM part of the proposal, i) next step split groups in NXcs_computer, ii) review remaining large EM base classes * Fix issues with NXcs_computer * Further work on EM base classes, added beam_blanker * Edits em-domain-specific methods * Fixing warning issues with the documentation building * Few more fixes and make local running fine now * Fix copyrights * Generate NXDL files * Fixing incorrect nameType * Revert change in Makefile * Updte custom dictionary cuz of new custom names * Eliminate remaining references to NXcoordinate_system_set * Finished NXem * Removing inconsistencies between fairmat version and NIAC proposed version of the EM pr * Make remaining nxdls that were not yet in sync * Cleaned nyaml and regenerated them from scratch * Whitespace changes on NXcg that were relevant for syncing with the NIAC * Brought back changes for atom probe proposal and a few fixes where appdefs and baseclasses where inconsistent with those from the NIAC on their current main ecc9361 * Brought (speculatively still because EM not yet voted upon but voting will start in two days and wont change fundamentally once voting starts) back changes for em proposal evaluating against fairmat-2024-em branch commit 54f6d29 * Brought back (speculatively) cg classes FAIRmat and pre-FAIRmat ones, also updated all XML headers from discouraged single to double straight quotes, all nyamls reprocessed clean * Brought back (speculatively, although voted but nameType for AXISNAME concepts in NXtransformations should be checked) changes from optical spectroscopy evaluated against fairmat-2024-optical_spectroscopy 6bba40f * Given that mpes, xps, apm and opt were standardized, and em, and cg in ancestor commits to this commit added either as they were accepted from the NIAC main or as they are speculatively assumed - to not further block modifications to the software tools - there are no remaining appdef changes that might still need some old FAIRamt contributed definitions. Therefore, this commit removes all yaml and nxdl files from all those classes that are no longer developed that have been promoted already to the base classes or appdefs or some classes which are no longer developed further, reasons were: i) removed because obsolete: NXion,roi,sample_component_set, ii) removed postpone for future: NXlab_electro...,lab_sample..., microstructure_gragles_..., microstructure_imm..., iii) remove promoted to appdef NXapm,ellipsometry,em,optical_spectroscopy,raman, iv) remove because promoted to base class, remaining of the removed * Refactoring of deprecated NXion to NXatom * Fixing math env error for rendering in rst for NXatom * Edits that take into account the state of upstream/main from the NIAC from 2025/07/17 as seen by the updated fairmat-2024-em branch for which the voting started today, next steps, a few touches on remaining contributed definitions * One more update from the NXem voting * Fixing an uncaught issue in NXsource * Edits on NXmicrostructure * Reactivated microstructure, edit NAMED_reference_frameID * Edits reintroducing microstructure for the AMAG test case * Added NXchemical_composition to NXmicrostructural_feature to allow for per feature composition reporting * Make microstructure a subordinate of phase * Removing inconsistency of explicit demanding for units in NXem_eds * Clarification of a nameType that should not be specified * Fix mentioning of removed class NXion * Further changes given that some NeXus classes have been removed * Fixes related to spellchecking, always sort the entries in the custom dictionary lexicographically ascending * Further spelling --------- Co-authored-by: mkuehbach <markus.kuehbach@physik.hu-berlin.de>
|
There are a few files that I believe still don't follow the naming convention for domain-specific base classes, i.e., NXcorrector_cs, NXevent_data_em, and NXinstrument_em. I also note that NXaberration is documented as specific to the 'em' domain - should it be renamed NXem_aberration or do we think it is applicable as is to other uses of aberration corrections? |
…en that the vote is running I prefer to decide about these updates in an immediate follow-up
|
Thank you @rayosborn comments: We should not change this in a PR that is currently in a voting process, a few thoughts on this for each suggestion follows, as the changes here asked for are not only on docstrings
@phyy-nx I suggest to make a follow up PR with the content of proposal: FAIRmat-NFDI#402 after the votes on this EM PR here have passed. @rayosborn is this a compromise you support? I have implemented a solution via the follow-up proposal:
|
|
Vote has passed. Thanks everyone!! |
|
agreement from Telco: we can merge this PR, and then make a new PR from FAIRmat-NFDI#402 and then merge that one, too. |
|
Last comment addressed in #1585 |
Essence of changes
Dependencies of this PR on other PRs
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:
#1410 (not used here anyway)
#1412 (but concepts not used in EM)
#1415#1407-> merged#1408-> merged#1413-> merged#1414-> merged#1419-> merged#1420-> merged#1486-> mergedEM-specific base classes
There are several new base classes that are currently only used in NXem, they are ordered in a sequence of dependencies to assure that reviewing adds building blocks gradually:
Conceptual modeling of aberrations that are relevant to assure the microscope is calibrated well enough to achive precision and accuracy and sharp images:
NXaberration
NXcorrector_cs
Base classes for talking about thermodynamic phases and crystal structure as used in NXem_ebsd
NXphase
NXunit_cell
Base classes for concepts and/or components of an EM microscope
NXebeam_column
NXevent_data_em
NXibeam_column
NXmanipulator (essentially the version from #1424)
NXoptical_system_em
NXscanbox_em
Specialized NXprocess instances for modularized reporting of parameter, configs, and methods for common imaging modalities and types of NXdata collected, postprocessing.
NXem_ebsd (that is the most detailed example given the large number of example data in the public domain)
NXem_eds
NXem_eels
NXem_img
Specializations of NXdata to not let NXem grow even larger but allow to reduce the number of possible combinations how images and spectra will be named to work towards a more homogeneous presentation of n-dimensional hyperslab data:
NXimage, NXspectrum
The appdef itself:
NXem, the application definition that uses currently all these concepts because of which ought to be reviewed together
Several edits were necessary in classes unrelated to NXem only for the purpose of making the docs compilable (these are e.g. NXapm, edits in domain-specific-structure rst files of the documentation, or tests that otherwise would have failed. With the changes being worked on in parallel to this PR, merging back the NIAC main (i.e. upstream/main from the perspective of the FAIRmat/nexus_definitions), ensure that these pragmatic edits will gradually become superseeded by the respective PRs.
Small base classes which in the development of NXem turned out as too insignificant/not meaty enough ones were reworked with their concepts merged e.g. in the NXem appdef:
- NXem_correlation-> removed, placed in NXem- NXem_method-> removed, placed in NXem- NXem_msr-> removed, placed in NXem- NXem_sim-> removed, placed in NXem- NXevent_data_em_set-> removed, placed in NXem- NXinteraction_vol_em-> removed, placed in NXemChanges for base classes within the realm of computational geometry
Are covered in #1421, #1529, their usage in NXem is planned and the places intentionally placed but commented out to disentangle these geometry-related PRs
NXcg_{point,triangle,*}classesLikewise, changes for base classes within the realm of documentation concepts for representing the geometry and properties of crystal defects and microstructures to bridge to our friends in materials engineering including appdefs for the simulation of microstructure evolution are planned to become usable in NXem but also those where here intentionally placed but commented out to also disentangle this set of classes from those for EM:
NXmicrostructure_{*}-> previously named as NXms_* classesFurther context is provided via a list of dependant changes, see #1465