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

Feature request: "DefaultedScanOption" #327

Open
hartytp opened this issue Jun 6, 2023 · 7 comments
Open

Feature request: "DefaultedScanOption" #327

hartytp opened this issue Jun 6, 2023 · 7 comments

Comments

@hartytp
Copy link
Contributor

hartytp commented Jun 6, 2023

When setting up a scan using the argument editor, one often wants one or more of the experiment parameters to be held at their default values. At present, the best fit that ndscan provides for this is the FixedScanOption, which holds a parameter at a constant user-specified value. This kind of does the job since when the argument editor is refreshed, all fixed scan options revert to the current evaluation of the parameter's default value. This is fine for parameters which default to a constant, but is a common footgun for parameters whose default is a dataset - if the argument editor is left open for a while, the datasets can change but that won't be reflected in the argument editor.

To some degree, this is a general ARTIQ problem, but I do wonder if we can do something to improve the quality of life for ndscan users. Proposal:

  • introduce a new DefaultedScanOption which always sets the parameter to whatever its default evaluates to when the scan starts. This should be the default scan option.
  • add a new SpanScanOption which behaves the same as the existing CentreSpanScanOption but always uses the parameter's current default value as the centre.
@dnadlinger
Copy link
Member

Maybe this could also be implemented as an UI widget that is greyed out/… by default. Perhaps it could even show "1.234 µs (dataset default)" or something in this greyed out fashion, so that the user has a hint of what the default value is likely going to be, but such that is also visually unambiguous that it will be overridden.

This could then be used for both the "fixed" option and the centre of centre-type scans.

@hartytp
Copy link
Contributor Author

hartytp commented Jun 6, 2023

Maybe this could also be implemented as an UI widget that is greyed out/… by default
This could then be used for both the "fixed" option and the centre of centre-type scans.

Do you have in mind something which can be toggled between held at default (greyed out with some hint displayed) and editable? How would we handle the toggling in the UI? Add a checkbox?

@hartytp
Copy link
Contributor Author

hartytp commented Jun 7, 2023

Perhaps it could even show "1.234 µs (dataset default)" or something in this greyed out fashion, so that the user has a hint of what the default value is likely going to be, but such that is also visually unambiguous that it will be overridden.

I was thinking about something similar. The one point I'd make is that I'm coming around to the view that no information is better than potentially incorrect information. e.g. it's easy for screen clips to end up in lab books and then mislead people.

One could imagine doing something like subscribing to the dataset_db and always displaying the latest value (re-evaluate when a linked dataset changes). But, then one has a bit of an ambiguity between "does the dashboard show me what value was used last time I ran this or what would be used if I clicked submit now".

It's not obvious to me how valuable seeing live values in the dashboard really is anyway so I'm somewhat inclined to not actually display any value. Maybe just show the dataset path?

@hartytp
Copy link
Contributor Author

hartytp commented Jun 8, 2023

One other comment: we're starting to have a lot of scan options to cover permutations of a small number of settings - linear v refining, centre/span v min/max, pull from default v manual entry (e.g. centre/span with the centre being a dataset). At some stage it might make sense to consider refactoring to have a few different options to cover these basics rather than manually supporting certain permutations.

@dnadlinger
Copy link
Member

One other comment: we're starting to have a lot of scan options

Exactly, hence the suggestion of merging the default behaviour into the other scan options by allowing users to just delete the contents of the text field (or some other intuitive option that doesn't take up much screen real estate).

@hartytp
Copy link
Contributor Author

hartytp commented Jun 8, 2023

aah, thanks for the clarification - I'd misunderstood what you had in mind.

I'm not sure what the best path is here. Having behavior of "leave the spin box empty to get the default value" feels like it could be rather accident prone, which I'm keen to avoid. But I do agree with you re the value of screen real estate.

@dnadlinger
Copy link
Member

Having behavior of "leave the spin box empty to get the default value" feels like it could be rather accident prone,

I don't think that's necessarily the case if the spin box is then highlighted in a different colour (grey/…), and a message of the default value being used displayed. Should be fairly obvious what's going on there.

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

No branches or pull requests

2 participants