outline of scan and grid_scan enhancement proposal

[awalter-bnl]

I am placing this here to gauge feedback, essentially I am proposing to match the approach taken in bluesky PR# 1194 for log_scan and log_grid_scan to scan and grid_scan .

In addition to reducing complexity in our code-base this will also (by default) resolve bluesky issue #1189

[danielballan]

One downside is that, in losing "outer product" and "inner product" plan patterns, one forfeits useful information about the structure of the data. Those plan patterns told us specifically that could expect linearly spaced steps, and it characterized them in a succinct form. The new form would spell out the individual steps always and throw away the information that the steps are linearly-spaced.

We could still infer the linear spacing, and access the start/stop/num parameters, from plan_name and plan_args, so I think this change would make "plan patterns" less useful but still leave us with the information we want.

[awalter-bnl]

so I think this change would make "plan patterns" less useful but still leave us with the information we want.

I agree with @danielballan on this point, it does make 'plan_patterns' potentially less useful. That being said I am not sure how much 'plan_patterns' is being used at the moment, and so perhaps they are not so useful in practice anyway. I think for simple plans like scan and grid using outer_list_pattern and inner_list_pattern is fine, I am less convinced for the more complicated scans like the spiral ones.

[danielballan]

Original author of plan patterns @klauer should weigh in here. Maybe also of interest to @tacaswell.

[klauer]

I'm biased here, of course.

I like having the plan patterns available for easy reconstruction of the user intent (even for a failed/cancelled scan, etc.). The patterns define the setpoints of the scanned device/signal - these are not always recorded and available after a scan.

While combining the point generation (plan patterns) with the plans themselves is compelling because it is simple, recreating the pattern may require duplicating that buried pattern creation code elsewhere.

[awalter-bnl]

While combining the point generation (plan patterns) with the plans themselves is compelling because it is simple, recreating the pattern may require duplicating that buried pattern creation code elsewhere.

While this point is true, in this case I am not suggesting including the point generation in the 'plans' themselves. We would still have a plan_pattern (inner_list_pattern or outer_list_pattern) and we also save, in the metadata, the arguments passed to these pattern functions. As a result it would be possible to 'recreate' the pattern from inner_list_pattern/ outer_list_pattern and the 'plan_pattern' metadata so in terms of later recreation it would not be any different from the current solution. The main difference is that we would have fewer 'plan_pattern' functions, which I think would be easier to maintain and reduce the amount of 'highly similar code'.

[danielballan]

in terms of later recreation it would not be any different from the current solution.

I think an important point might either be missed or glossed over here. Currently scan([det], motor, -1, 1, 10) stores -1, 1, and 10 and the information that this is a linearly-spaced pattern. If we refactor scan to use the same pattern as list_scan, we will then be storing the full list of positions array([-1. , -0.77777778, -0.55555556, -0.33333333, -0.11111111, 0.11111111, 0.33333333, 0.55555556, 0.77777778, 1. ]), and we will lose the information that this was a linearly-spaced pattern. We will be left with metadata that is more verbose and less informative.

So the question is whether the reduction in highly similar code advocated by @awalter-bnl is worth this small regression in functionality. Agreed?

[tacaswell]

I think it is possible to for us to inject the current "nice" meta-data in a way that propagates through in a clean way to get the best of both worlds (the 'close-to-the-user' metadata / plan_patterns) and minimizing code duplication.

If it does not overlay cleanly (due to miss-matched keys and what not) we could pull some private _list_base out the bottom.

We should add some tests on the meta-data coming out of plans to make this sort of re-factor safer in the future!

[prjemian]

re comment by @danielballan, don't we have the metadata already in plan_args about the type of scan (and thus constant step size)? Seems we have this but it is up to a plan to add this metadata tag and also requires the consumer of this info to parse it in this tag relative to the plan. That is, one must understand what args a given plan uses.

[awalter-bnl]

@prjemian is correct, the information relating to the fact that this plan is linearly spaced, and what the spacings are, is still specified in the plan_args metadata. This approach would only change the plan_pattern_args.

[danielballan]

Yes, but the reason for adding "plan patterns" in the first place is that visualizers etc. could refer to them without going to plan_args (which invoke more bluesky-specific concepts). The information that the scan is linearly spaced wouldn't be lost, but it would effectively break the utility of plan patterns for this use case.

[ronpandolfi]

I like the parity between grid_scan and a standard image display. I'd be concerned that backing away from the restriction of a uniform rectangular grid could make visualization more involved.

[klauer]

I think it's acceptable for bluesky itself to take up the burden of maintaining duplicated code if it benefits the user in the end, especially in the case of useful metadata and reducing downstream code complexity.

That said, I don't feel strongly enough about this to completely reject the suggestion and would defer to others.

[danielballan]

This has stalled.

I think we agree that, as a point of fact, this code simplification would come at a small but notzero loss of information for document consumers: namely, they would get a list of positions instead of a range and an indication of linear spacing. They would have to consult plan_args, taking into account more bluesky-specific semantics and somewhat defeating the point of plan_pattern, or else infer linear spacing from a list of points.

If we were starting from a blank slate, I think there is a fair argument that we should have left the code simpler. But I think the argument for stripping this out now that it has already been written, is not very strong, considering the low cost of maintaining this code and the possibility that the simplification could break things.

Is anyone championing this proposal, or shall we close?

[awalter-bnl]

@danielballan, I still think this change is worth it but am not willing to dig in any further. If the consensus is to not proceed then it can be closed.

[awalter-bnl]

judging by the lack of response in the last few days I assume the consensus is not to make this change. closing it out.