Connect more intricate concepts from NXmicrostructure#406
Conversation
Reviewer's GuideThis PR enriches the NXem schema by embedding detailed NXmicrostructure, NXmicrostructure_odf, and NXmicrostructure_pf groups (with full configuration, parameter, geometry and feature subgroups), removes redundant nameType constraints on AXISNAME_indices, and updates related contributed definitions (adding texture characteristics, refining documentation and base types). ER diagram for new and updated microstructure-related entitieserDiagram
NXem ||--o{ NXmicrostructure : contains
NXem ||--o{ NXmicrostructure_odf : contains
NXem ||--o{ NXmicrostructure_pf : contains
NXmicrostructure ||--|| NXprocess : configuration
NXmicrostructure ||--o| NXcg_grid : grid
NXmicrostructure ||--o| NXcg_point : points
NXmicrostructure ||--o| NXcg_polyline : polylines
NXmicrostructure ||--o| NXcg_triangle : triangles
NXmicrostructure ||--o| NXcg_polygon : polygons
NXmicrostructure ||--o| NXcg_polyhedron : polyhedra
NXmicrostructure ||--o| NXmicrostructure_feature : crystals
NXmicrostructure ||--o| NXmicrostructure_feature : interfaces
NXmicrostructure ||--o| NXmicrostructure_feature : triple_junctions
NXmicrostructure ||--o| NXmicrostructure_feature : quadruple_junctions
NXmicrostructure_odf ||--|| NXparameters : configuration
NXmicrostructure_odf ||--o| NXprocess : characteristics
NXmicrostructure_odf ||--o| NXdata : phi_two_plot
NXprocess ||--o| NXnote : source
NXprocess ||--o| NXdata : roi
NXprocess ||--o| NXprogram : programID
NXprogram ||--o| NXmicrostructure_mtex_config : mtex
NXmicrostructure_mtex_config ||--|| NXparameters : extends
Class diagram for enriched NXem schema with detailed microstructure conceptsclassDiagram
class NXem {
+indexing: NXprocess
+odfID: NXmicrostructure_odf [0..*]
+pfID: NXmicrostructure_pf [0..*]
+microstructureID: NXmicrostructure [0..*]
}
class NXprocess {
+number_of_scan_points: NX_UINT
+indexing_rate: NX_NUMBER
+source: NXnote
+roi: NXdata
}
class NXmicrostructure {
+configuration: NXprocess
+grid: NXcg_grid
+points: NXcg_point
+polylines: NXcg_polyline
+triangles: NXcg_triangle
+polygons: NXcg_polygon
+polyhedra: NXcg_polyhedron
+crystals: NXmicrostructure_feature
+interfaces: NXmicrostructure_feature
+triple_junctions: NXmicrostructure_feature
+quadruple_junctions: NXmicrostructure_feature
}
class NXmicrostructure_feature {
+representation: NX_CHAR
+number_of_crystals/number_of_interfaces/number_of_junctions: NX_UINT
+index_offset: NX_INT
}
class NXmicrostructure_odf {
+configuration: NXparameters
+characteristics: NXprocess
+phi_two_plot: NXdata
}
class NXparameters {
+crystal_symmetry_point_group: NX_CHAR
+specimen_symmetry_point_group: NX_CHAR
+kernel_halfwidth: NX_NUMBER
+kernel_name: NX_CHAR
+resolution: NX_NUMBER
}
class NXprocess_odf {
+texture_index: NX_FLOAT
}
class NXmicrostructure_mtex_config {
<<extends NXparameters>>
+conventions: NXcollection
}
NXem --> NXprocess : indexing
NXem --> NXmicrostructure_odf : odfID
NXem --> NXmicrostructure_pf : pfID
NXem --> NXmicrostructure : microstructureID
NXmicrostructure_odf --> NXparameters : configuration
NXmicrostructure_odf --> NXprocess_odf : characteristics
NXmicrostructure_odf --> NXdata : phi_two_plot
NXmicrostructure --> NXprocess : configuration
NXmicrostructure --> NXcg_grid : grid
NXmicrostructure --> NXcg_point : points
NXmicrostructure --> NXcg_polyline : polylines
NXmicrostructure --> NXcg_triangle : triangles
NXmicrostructure --> NXcg_polygon : polygons
NXmicrostructure --> NXcg_polyhedron : polyhedra
NXmicrostructure --> NXmicrostructure_feature : crystals
NXmicrostructure --> NXmicrostructure_feature : interfaces
NXmicrostructure --> NXmicrostructure_feature : triple_junctions
NXmicrostructure --> NXmicrostructure_feature : quadruple_junctions
NXprocess --> NXnote : source
NXprocess --> NXdata : roi
NXmicrostructure_mtex_config --|> NXparameters
Class diagram for NXmicrostructure_mtex_config base type changeclassDiagram
class NXmicrostructure_mtex_config {
<<extends NXparameters>>
+conventions: NXcollection
+other attributes...
}
NXmicrostructure_mtex_config --|> NXparameters
Class diagram for new characteristics in NXmicrostructure_odfclassDiagram
class NXmicrostructure_odf {
+characteristics: NXprocess
}
class NXprocess {
+texture_index: NX_FLOAT
}
NXmicrostructure_odf --> NXprocess : characteristics
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @mkuehbach - I've reviewed your changes - here's some feedback:
- Consider splitting this large schema update into smaller, feature-focused PRs (e.g. microstructure, ODF, PF) to make review and testing more manageable.
- Ensure consistency across the new groups and attributes—especially around nameType and exists/minOccurs/maxOccurs definitions—so they align with existing Nexus conventions.
- Verify that switching NXmicrostructure_mtex_config from extending NXobject to NXparameters won’t break downstream compatibility and document any breaking changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider splitting this large schema update into smaller, feature-focused PRs (e.g. microstructure, ODF, PF) to make review and testing more manageable.
- Ensure consistency across the new groups and attributes—especially around nameType and exists/minOccurs/maxOccurs definitions—so they align with existing Nexus conventions.
- Verify that switching NXmicrostructure_mtex_config from extending NXobject to NXparameters won’t break downstream compatibility and document any breaking changes.
## Individual Comments
### Comment 1
<location> `contributed_definitions/nyaml/NXmicrostructure.yaml:228` </location>
<code_context>
<doc>
Reference to an instance of:
- * :ref:`NXcg_polyline` for a one-dimensional representation as only a projection is available (like in linear intercept analysis)
- * :ref:`NXcg_polygon` for a two-dimensional representation as only a projection is available (like in most experiments)
- * :ref:`NXcg_polyhedron` or :ref:`NXcg_grid` for regularly pixelated (in 1D, 2D) or voxelated representations (in 3D)
+ * :ref:`NXcg_polyline` for a one- or two-dimensional representation as only a projection is available (like in linear intercept analysis)
+ * :ref:`NXcg_polygon`, :ref:`NXcg_triangle`, or :ref:`NXcg_polyhedron` for a two- or three-dimensional representation as only a projection is available (like in most experiments)
+ * :ref:`NXcg_grid` for regularly pixelated (in 1D, 2D) or voxelated representations (in 3D)
</code_context>
<issue_to_address>
The updated description for NXcg_polyline may cause confusion regarding its dimensionality.
Please clarify whether 'one- or two-dimensional' refers to the geometry of the polyline or its embedding space, as this could be misinterpreted by users.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
* :ref:`NXcg_polyline` for a one- or two-dimensional representation as only a projection is available (like in linear intercept analysis)
=======
* :ref:`NXcg_polyline` for a representation of a polyline embedded in one- or two-dimensional space, as only a projection is available (like in linear intercept analysis)
>>>>>>> REPLACE
</suggested_fix>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
I am not sure where we are going with this. Is this supposed to be an addition to the contributed definitions? Then Or is the |
|
Different levels of standardization possible. An update of an appdef that includes concepts not yet standardized can still be used such that it obeys the voted upon standard. Relevant is the combination of instance data populated in the tree and evaluated against the tree, like a lower bound constrain on a functionality. Its the clarity in the communication which matters the wording must be we combined a standardized reporting for data collected with the microscope (covered by the parts of the concept tree in NXem) with additional supplementary concept tree branches that are not yet standardized but meant as an example how to achieve such domain-level standardization with NeXus. This PR summarizes the changes and idea that we withhold from the NIAC standardization and here add on top of the standardized NXem. nexusformat#1423 had NXmicrostructure, CG, IPF, ODF intentionally not included. As asked by Aaron, "what will be the first thing that you'll change once 1423 is accepted?" The answer on our side was: "the aforementioned (microstructure, cg) will be added". Standardized of the concept tree in NXem is right now what is covered by 1423, eventually at some point #402. In addition, NXem provides FAIRmat-specific extensions for aforementioned microstructure, ipf, and odf. The CG concepts here used are standardized (nexusformat#1421). What means and is standardized? So far the discussion was only what is exactly reported as demanded by the voted upon NXem likewise for other the appdef used. Reporting of the EM database examples follows the standard, will though contain additional (meta)data instances. Also these instance data are passed to the validation instead of just dropping these into a loose NXcollection. No further additions functionality-wise are planned. |
|
Submitting the NXmicrostructure to the NIAC with aiming for more than updating the existent and quite outdated definitions in contributed as of now is a clear "no" from my side. I would like to mention that here hooked in semantic additions bridge physics-with-engineering going beyond what was reported in other NFDI consortia and applying the same quality assurance when it comes to validation of instance data. This is mainly to lead by example to invalidate the point that "this is not possible with NeXus" --- it is but the set also explores the current limits of NeXus as likewise the entirety of the FAIRmat contributions. |
My understanding from NIAC was always that application definitions that are part of the standard (i.e., voted on and moved to |
lukaspie
left a comment
lukaspie
left a comment
There was a problem hiding this comment.
LGTM.
As discussed, this introduces a difference to the accepted NXem in the NIAC standard. We should work towards a resolution. Options:
- We propose all microstructure classes to be standardized (not desired)
- We add all microstructure classes to the NIAC as contributed, but do not propose the changes to
NXemto NIAC. We then have to decide what we want to do aboutNXemin thefairmat. We either keep these optional changes or we make changes to the validation/NOMAD parsing such that if we don't find a given concept in the appdef or in its inheritance tree, we also check all other base classes. I would vote for the latter option.
3 participants
exemplified for EBSD:
Summary by Sourcery
Extend the NXem schema to fully integrate microstructure, ODF, and pole figure data for EBSD workflows by adding repeating groups for microstructureID, odfID, and pfID, while refining related definitions and documentation.
New Features:
Enhancements: