Skip to content
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

Base class inheritance example #66

Draft
wants to merge 7 commits into
base: fairmat
Choose a base branch
from

Conversation

domna
Copy link

@domna domna commented Sep 13, 2023

This is an example for NXbeam to discuss and implement an example for base class inheritance.

I roughly divided the current NXbeam into multiple sub-classes. Feel free to suggest moving or adding fields. Also if you have an idea how to build a multiple inheritance example from this feel free to suggest something.

This is the current structure:

graph TD
    A[NXbeam] --> B[NXbeam_polarized]
    A --> F[NXbeam_optical]
    A --> G[NXbeam_electron]
    F --> H[NXbeam_xray]
    A --> C[NXbeam_neutron]
    A --> D[NXbeam_time_resolved]
    B --> E[NXbeam_polarized_time_resolved]
    D --> E
Loading

The NXbeam_polarized_time_resolved example doesn't really make sense. I just added it to show a possible formulation of multiple inheritance.

@sherjeelshabih @Tommaso-Pincelli @tomio13 @mkuehbach @rettigl @sanbrock @RubelMozumder

Copy link
Collaborator

@mkuehbach mkuehbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @domna here my thoughts

base_classes/nyaml/NXbeam_optical.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXbeam.yaml Outdated Show resolved Hide resolved
@@ -0,0 +1,258 @@
doc: |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For each docstring we should have at least the optional possibility but a field and keyword/command for this realized whereby we can add a reference to a concept of another ontology (OBO, BFO, EMMO) ...

Also the docstrings should really be inspected and cleaned from instructing to humans what they should format there, if we feel we need to express how things should be formatted behind a field we talk about constraint management

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I support this. We could add something like ontology_reference or ontology_uri as sub-field of doc?

I like the idea of adding constraints (I'm thinking of describing formulas etc. properly here as well) but I have the feeling that currently its a bit out of our scope of our simple prototype example here.

base_classes/nyaml/NXbeam.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXbeam_neutron.yaml Outdated Show resolved Hide resolved
base_classes/nyaml/NXbeam_neutron.yaml Outdated Show resolved Hide resolved
Properties of an optical beam at a given location.
category: base
(NXbeam)NXbeam_optical:
incident_polarization(NX_NUMBER):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would tend to move this to NXbeam_polarization and then add in this polarization base class the method whereby that polarization is described/parameterized e.g.
descriptor(NXCHAR):
enumeration: [stokes]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this to NXbeam_polarized.

Could you elaborate on the described/parametrized notation you propose? I do not understand this.

base_classes/nyaml/NXbeam_optical_time_resolved.yaml Outdated Show resolved Hide resolved
chirp_GDD(NX_FLOAT):
unit: NX_TIME
doc: |
Group delay dispersion of the pulse for linear chirp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a linear chirp there is likely also a nonlinear chirp, would a base class NXsignal_processing_descriptor_chirp make sense (dont take that naive name of this base class for granted) but read "between the lines" conceptually?

Same story for frog?

I see one problem/inconvenience of a finer granularization maybe people need to navigate deeper but the benefit is stronger modularization, I am still not convinced though that I am overdoing it here with the balance between convenience and granularization

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants