NXstress I/O#967
Merged
Merged
Conversation
ekapadi
force-pushed
the
EWM12482_NeXus_prototype
branch
10 times, most recently
from
973b4a3 to
8099203
Compare
Contributor
|
Did we ever get the cis test script for this? |
ekapadi
force-pushed
the
EWM12482_NeXus_prototype
branch
from
0d049ec to
247a73c
Compare
* For an overview of these changes: please run `pixi run build-dev-docs` and then see:
`docs/_build/developer/design/nexus/IO_prototype.html`
(built from: `docs/developer/source/design/nexus/IO_prototype.rst`).
* Round-trip test script: `tests/scripts/cis_tests/NXstress_demo_script.py`.
* Also fixed `HidraProjectFile` reload so that it places the variance data at the correct location in `HidraWorkspace`.
ekapadi
force-pushed
the
EWM12482_NeXus_prototype
branch
from
b4a2257 to
4cab5d9
Compare
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Contributor
|
The output of the test script when I ran it (NOTE: Step 3 seemed to take a while, and slowly wrote to disk): |
Contributor
|
With a little ai assistance, I am able to interpret these results as meaning a successful round trip with little to no drift in the data. I would ask that some sort of wire/flow diagram in mermaid be added to help clarify the structure of the new code, as its fairly large. If there was something to visually layout Consts - pyrs/utilities/NXstress/_definitions.py and in addition to that I guess there were a couple ui changes/backend changes to support the i/o. like this peak prof util? should these bullets have an issue associated with them? Sorry if I missed where you explained it but whats up with these try and ignore error lines in the ui code? |
Contributor
Author
Considering the state of PyRS code-base itself (which isn't too hot), I'd hesitate to put anything in a sub-package. And yes, writing the input data takes a long time, but I will note that the input data is not required by the NXstress output implementation. :) |
walshmm
approved these changes
May 13, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
NeXus IO prototype
Overview
The objective of the NeXus IO-prototype work was to provide a first-pass
implementation of NeXus-compliant output using the existing
NXstressschema. Both output, and the corresponding input methods are now
implemented. Primarily, the classes
HidraProjectFile,HidraWorkspace, andPeakCollectionare used to obtain informationabout the reduced data. Supporting classes such as
SampleLogs, andInstrumentSetupare also used, where information about the instrumentand the experiment specifics is required.
A sub-objective was to provide an output format that could include all
of the data associated with an experiment, such as input-data and any
additional normalization spectra. It should be noted however, that
including this information is optional with respect to the
NXstressschema.(Also, as an alternative, it is quite common to simply specify
input-data by noting the file-names in appropriate fields. The problem with this approach
is that file-names usually will only have meaning at the originating site.)
The next sections provide a correspondence between the python classes,
and sections within the
NXstressschema. Any place where there's stillconfusion, or there is simply not enough information to meet the
requirements of the schema, will be indicated using bold text.
For purposes of the prototype, the
nexusformatpython package is usedin the implementation, and that working group's validator has been used
for validation of compliance. With respect to validation, its important
to use a validator that allows overriding NeXus base-class
definitions, which
NXstressdoes extensively. In this regard, NeXusInternational Advisory Committee's (NIAC) C-language validator is an
incomplete implementation, and gives misleading results. Also noted
were several bugs in the implementation of the
nexusformatvalidator. For example, this validator would not properly validate role-specified groups (noted as
UPPERCASEin the schema), whichNeXusspecifically allows to have any desired name.In this case the validator actually requires
UPPERCASE, and won't allow custom names.Primary
NXentrygroupIssues found:
scan-point (aka subrun number in PyRS). We could alternatively use
the minimum and maximum over all of the sub-run times to obtain
these values. The validator has trouble with the placement of
lists for these fields, but technically our use of lists should
be compliant with
NXstress, and this is almost certainly an additionaldefect with the validator implementation itself
Single
PEAKSgroupThis group is intended to contain the canonical (or reference) peak
values.
Issues found:
PEAKSgroup: only a single PEAKS group is allowed by theNXstressschema. This meant that in order to include multiplePeakCollectionwe needed to use a flattened-indexing scheme.Such a scheme is allowed by the
NXstress-schema, but it is highlyunlikely that any generic
NeXusapplication will be able to decodethese indices automatically. Further, since any data reduction
(and associated peak fitting) would normally depend on the mask
used, a *mask* field has been added to ``PeakCollection``.
In order to not overly modify the existing code, this
maskfieldis optional, with a default value corresponding to the default
mask.
and ``(h, k, l)`` (Miller indices) tags. At present we make
this conversion automatically using a regular-expression based
parser, however this is not an ideal solution. Here it would be
better if these values were specified explicitly by PyRS as
separate fields in the
PeakCollection.required values to include in this section.
because the logs had the same variable names -- this is probably
incorrect!
components of the normalized scattering vector Q in the sample
reference frame)**. These seem to have no correspondance in the
current PyRS codebase -- these values are initialized to
NaN.FIT(NXprocess) groupThis group contains the fitting results from the selected peak-profile
/ background-function combination. In order to include results from
multiple
PeakCollection, this group uses the identicalflattened-indexing scheme as the
PEAKSgroup. Note that thePEAKSgroup includes all of the field-values which define this index, and
those values are not repeated in the
FITgroup.PeakCollectionfields betweenFITandPEAKSsubgroups fromNXstresswas a bit confusing. This needs tobe examined carefully to determine if it is correct.
FIT/DIFFRACTOGRAM/fit,fit_errors: these datasets should containthe reconstructed spectrum from the fitted model. We don't seem to
have methods to do this yet, so these are initialized to NaN.
SAMPLE(NXsample) groupThis was complicated! Again the main issue is the naming of things in
NXstressvs. the naming in the PyRS codebase.Issues found:
this correct?
have a single value for the entire experiment. This still needs to
be checked log-by-log!
included in an additional
logs(NXcollection) subgroup.INSTRUMENT(NXinstrument) groupIssues found:
solid-angle masks. By necessity, this treatment assumes that a
<default>mask will always exist, and this mask is created atoutput when necessary. Detector and solid-angle masks are
distinguished by their array shape. Note that under the current
mask-naming scheme used by PyRS, masks must have *distinct* names,
regardless of type.
treated, and this will require some additional work in order to make
sure that the treatment is correct. In PyRS itself, several bugs
were found relating to how calibration is applied: specifically,
there's nothing preventing it from being applied multiple times.
in the instrument section!
Possible extensions
of the monochromator information.
changed so that it is explicit. At present, distinguishing between
a detector or solid-angle mask depends only on array shape.
Check list for the pull request
Check list for the reviewer
Manual test for the reviewer
In addition to running the unit tests, the complete
NXstress-I/O implementation can be verified using the script attests/scripts/cis_tests/NXstress_demo_script.py. Note that this script needs to be run onanalysis.sns.gov(or a system that has the/HFIRmount).References