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

Reflectometry Amor workflow in Sciline #5

Closed
wants to merge 18 commits into from
Closed

Reflectometry Amor workflow in Sciline #5

wants to merge 18 commits into from

Conversation

jokasimr
Copy link
Contributor

@jokasimr jokasimr commented Oct 20, 2023

This is not really ready to merge yet but I have some questions so I'll ask for review anyway.

Outstanding issues:

  • Some procedures have several optional arguments with default values, see supermirror_calibration and make_beamline. How should we deal with those parameters?
    • If we make each parameter into a provider that is going to clutter the graph visualization.
    • We could pass them as parameters to the pipeline. But how can we make them default parameters of the pipeline?
  • Orso
  • There's a separation between the Amor specific providers and generic reflectometry providers. Currently all domain types are defined in reflectometry.types but it would be better to specify the Amor specific types separately. How can this be done in a good way?
    • Consider for example the situation where reflectometry defines two nodes, A->B and the Amor workflow wants to place a step C in between them: A->C->B.

@jokasimr jokasimr marked this pull request as draft October 20, 2023 08:21
@jokasimr jokasimr closed this Oct 26, 2023
@jokasimr
Copy link
Contributor Author

Replaced by #10

@SimonHeybrock SimonHeybrock deleted the pair branch October 26, 2023 09:37
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