-
Notifications
You must be signed in to change notification settings - Fork 57
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
FAIRmat 2024: additional base classes in NXinstrument #1419
base: main
Are you sure you want to change the base?
FAIRmat 2024: additional base classes in NXinstrument #1419
Conversation
64226dd
to
5f5b269
Compare
base_classes/NXhistory.nxdl.xml
Outdated
<doc> | ||
Any activity that was performed on the physical entity prior or during the experiment. In | ||
the future, if there is base class inheritance, this can describe any activity, | ||
including processes and measurements. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the comment on the future
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXhistory.nxdl.xml
Outdated
subentry(NXsuxbentry): | ||
doc: | | ||
Any activity that was performed on the physical entity prior or during the experiment. | ||
definition: ["NXactivity"]--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXhistory.nxdl.xml
Outdated
<doc> | ||
Any chemical process that was performed on the physical entity prior or during the | ||
experiment. | ||
</doc> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove physical and chemical process as that will be covered by the NXactivity
base_classes/NXhistory.nxdl.xml
Outdated
persistent) identifier of e.g. another file which gives | ||
as many as possible details of the history event. | ||
</doc> | ||
</group> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove NXidentifier
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXhistory.nxdl.xml
Outdated
as many as possible details of the history event. | ||
</doc> | ||
</group> | ||
<!--There should be more activities here, like measurement.--> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXhistory.nxdl.xml
Outdated
reference to the location or a unique identifier or other metadata file. In the | ||
case these are not available, free-text description. | ||
This should only be used in case that there is no rigorous description | ||
using the base classes above. This field can also be used to pull in any activities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using the base classes above. This field can also be used to pull in any activities | |
using the base classes above. This group can also be used to pull in any activities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
base_classes/NXhistory.nxdl.xml
Outdated
</doc> | ||
</group> | ||
<!--There should be more activities here, like measurement.--> | ||
<group name="notes" type="NXnote"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove name="notes"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced by <group type="NXnote" nameType="any">
case these are not available, free-text description. | ||
This should only be used in case that there is no rigorous description | ||
using the base classes above. This field can also be used to pull in any activities | ||
that are not well described by an existing base class definition. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment that any number of NXnotes are allowed for extra details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
||
This class is planned be used in the future as the super class for all other activities if inheritance | ||
in base classes is supported in NeXus. | ||
</doc> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove reference to the future. "This class is a super class..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXactivity.nxdl.xml
Outdated
</doc> | ||
<field name="start_time" type="NX_DATE_TIME"> | ||
<doc> | ||
ISO 8601 formatted time code (with local time zone offset to UTC information |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove ISO reference. "Start time of activity"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inclusion of time zone should be a recommendation, even a strong recommendation but not a requirement.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented as
"Start time of this activity. It is recommended to include local time zone information."
<doc> | ||
ISO 8601 formatted time code (with local time zone offset to UTC information | ||
included) when this activity ended. | ||
</doc> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove ISO reference. "End time of activity"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See start time
base_classes/NXactivity.nxdl.xml
Outdated
Short description of the activity. | ||
</doc> | ||
</field> | ||
<group name="notes" type="NXnote"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove name="notes"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
ISO 8601 formatted time code (with local time zone offset to UTC information | ||
included) when this activity ended. | ||
</doc> | ||
</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add end_time_estimated
or add an estimated attribute on end_time
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added as
<attribute name="estimated" type="NX_BOOLEAN">
<doc>
In some cases, the end time of an activity can only be estimated. In this case,
this attribute shall be True.
</doc>
</attribute>
This can be any data or other descriptor acquired during the activity | ||
(NXnote allows to add pictures, audio, movies). Alternatively, a | ||
reference to the location or a unique identifier or other metadata file. In the | ||
case these are not available, free-text description. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add comment that any number of NXnotes are allowed for extra details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXresolution.nxdl.xml
Outdated
</field> | ||
<field name="relative_resolution" type="NX_FLOAT" units="NX_ANY"> | ||
<doc> | ||
Ratio of the resolution at a specified measurand value to that measurand value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ratio of the resolution at a specified measurand value to that measurand value, | |
Ratio of the resolution at a specified measurand value to that measurand value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
base_classes/NXresolution.nxdl.xml
Outdated
</group> | ||
<field name="formula_SYMBOL" type="NX_CHAR"> | ||
<doc> | ||
A symbol linking to another path in this appdef to be referred to from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be "A symbol linking to another path in the NeXus tree to be referred to from the..."
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
base_classes/NXresolution.nxdl.xml
Outdated
</group> | ||
<field name="formula_SYMBOL" type="NX_CHAR"> | ||
<doc> | ||
A symbol linking to another path in this appdef to be referred to from the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spell out appdef
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See above, changed to "A symbol linking to another path in the NeXus tree to be referred to from the..."
base_classes/NXresolution.nxdl.xml
Outdated
<field name="physical_quantity"> | ||
<doc> | ||
The physical quantity of the resolution, e.g., | ||
energy, momentum, time, etc. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we include spatial resolution as an example (e.g. imaging techniques)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added "area" to the examples
base_classes/NXinstrument.nxdl.xml
Outdated
<group type="NXinsertion_device" /> | ||
<group type="NXmirror" /> | ||
<group type="NXmoderator" /> | ||
<group type="NXmonochromator" /> | ||
<group type="NXpolarizer" /> | ||
<group type="NXpositioner" /> | ||
<group type="NXresolution"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove NXresolution from NXinstrument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
base_classes/NXresolution.nxdl.xml
Outdated
</field> | ||
<field name="relative_resolution" type="NX_FLOAT" units="NX_ANY"> | ||
<doc> | ||
Ratio of the resolution at a specified measurand value to that measurand value, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
measurand -> measured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We intentionally used measurand
here because we are not talking about measured values, but rather what resolution we will have given any value of a given measurand.
base_classes/NXresolution.nxdl.xml
Outdated
</field> | ||
<group name="response_function" type="NXdata"> | ||
<doc> | ||
The response of the instrument or part to a infinitesimally sharp input signal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The response of the instrument or part to a infinitesimally sharp input signal | |
The response of the instrument or part of the instrument to a infinitesimally sharp input signal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed
base_classes/NXresolution.nxdl.xml
Outdated
in the input axis or grid. | ||
This field should have the same dimensions as `input`. | ||
</doc> | ||
</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove input and magnitude, and expand doc string to describe how this should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
entered by the `formula_...` fields. | ||
The output unit should match the provided unit of this field. | ||
</doc> | ||
</field> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove formula_SYMBOL and change resolution_formula to resolution_description, update doc to indicate an english description of the math used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually use NXparameters for formula_SYMBOL
base_classes/NXcalibration.nxdl.xml
Outdated
calibration method but no actual calibration data. | ||
</doc> | ||
</group> | ||
<group name="calibration_reference" type="NXidentifier"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be replaced by identifier attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changed to <attribute name="identifier_calibration_reference">
base_classes/NXcalibration.nxdl.xml
Outdated
The axis values may be copied or linked in the appropriate NXcalibration fields for reference. | ||
</doc> | ||
</group> | ||
<group name="calibration_object" type="NXserialized"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use NXnote instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Dev notes to keep track:
depends on / blocked by:
|
47d5f4a
to
ea5c4f5
Compare
# Conflicts: # base_classes/NXaperture.nxdl.xml
… version of yaml. Removing unintensional comments # Conflicts: # base_classes/NXbeam.nxdl.xml # base_classes/NXdetector.nxdl.xml # base_classes/NXentry.nxdl.xml # base_classes/NXinstrument.nxdl.xml # base_classes/NXprocess.nxdl.xml # base_classes/NXsample.nxdl.xml # base_classes/NXsource.nxdl.xml # contributed_definitions/NXcollectioncolumn.nxdl.xml # contributed_definitions/NXmpes.nxdl.xml
# Conflicts: # contributed_definitions/NXmpes.nxdl.xml # contributed_definitions/nyaml/NXmpes.yaml
# Conflicts: # base_classes/NXenvironment.nxdl.xml # base_classes/nyaml/NXinstrument.yaml # contributed_definitions/NXmpes.nxdl.xml # contributed_definitions/nyaml/NXmanipulator.yaml # contributed_definitions/nyaml/NXmpes.yaml
# Conflicts: # base_classes/NXdetector.nxdl.xml # base_classes/NXsensor.nxdl.xml # base_classes/NXsource.nxdl.xml
# Conflicts: # base_classes/NXinstrument.nxdl.xml
# Conflicts: # base_classes/nyaml/NXinstrument.yaml
…ersion # Conflicts: # applications/NXarpes.nxdl.xml # applications/nyaml/NXarpes.yaml # base_classes/NXaperture.nxdl.xml # base_classes/NXbeam.nxdl.xml # base_classes/NXdata.nxdl.xml # base_classes/NXdetector.nxdl.xml # base_classes/NXentry.nxdl.xml # base_classes/NXenvironment.nxdl.xml # base_classes/NXinstrument.nxdl.xml # base_classes/NXmonochromator.nxdl.xml # base_classes/NXroot.nxdl.xml # base_classes/NXsample.nxdl.xml # base_classes/NXsample_component.nxdl.xml # base_classes/NXsensor.nxdl.xml # base_classes/NXsource.nxdl.xml # base_classes/NXsubentry.nxdl.xml # base_classes/NXtransformations.nxdl.xml # base_classes/NXuser.nxdl.xml # base_classes/nyaml/NXaperture.yaml # base_classes/nyaml/NXbeam.yaml # base_classes/nyaml/NXdata.yaml # base_classes/nyaml/NXdetector.yaml # base_classes/nyaml/NXentry.yaml # base_classes/nyaml/NXenvironment.yaml # base_classes/nyaml/NXmonochromator.yaml # base_classes/nyaml/NXprocess.yaml # base_classes/nyaml/NXroot.yaml # base_classes/nyaml/NXsample.yaml # base_classes/nyaml/NXsample_component.yaml # base_classes/nyaml/NXsensor.yaml # base_classes/nyaml/NXsource.yaml # base_classes/nyaml/NXsubentry.yaml # base_classes/nyaml/NXtransformations.yaml # base_classes/nyaml/NXuser.yaml # contributed_definitions/nyaml/NXinstrument.yaml
move NXfabrication to base_classes pull out base classes cited in NXinstrument
Co-authored-by: Aaron S. Brewster <[email protected]> Co-authored-by: Thomas A Caswell <[email protected]>
ea5c4f5
to
d79ccf5
Compare
Note that the CI/CD is failing here because we are bringing in several new base classes, which by themselves cite additional new base classes. This PR will only be ready once a decision on these new base classes has been reached.