-
Notifications
You must be signed in to change notification settings - Fork 23
Description
Overview
At present the ordering of steps during build / execution - init_params, get_default_analyses, collect_result_channels broadcast_metadata - is not documented and depends on the scan runner. I would find it helpful to document this and see if we can establish a consistent ordering. In particular, I'd like to see if we can establish an ordering where get_default_analyses is always called after init_params but before collect_result_channels.
Motivation
I've found it important to be able to make analyses depend on parameter values - for example, supposing I want to seed/constrain a fit parameter based on the value of an experiment parameter. That relies on the parameters being initialized before get_default_analyses (bit of a misnomer at this point?) is called.
If fragments are run using a TopLevelRunner this works fine. However, for subscans there is a bit of a rub because the subscans need to forward analysis results to result channels. This is achieved by calling get_default_analyses during build_fragment (as part of setup_subscan). At this point the parameters have not yet been initialized.
This is somewhat similar to the issues ARTIQ has around calling build multiple times - once for discovery and once to actually build the code. Arguable, calling these functions to discover parameters is a bit of a hack and we'd probably be better off using a more declarative approach - parameters / results / analyses are declared as class-level attributes and then we use introspection for discovery. But that's a bigger change to ARTIQ/ndscan.
Proposed solution
- Create and document an ordering of steps during build, which is consistent regardless of how the fragment is run.
- Ensure that in all cases, parameter initialization happens first, then
get_default_analysisthen result channel collecting (this requires small modifications to both TLR and subscans): modify the subscan code to create the analysis schema / result channels insideget_default_analysesinstead of insidebuild_fragmentmodify the TLR to callget_default_analysesbefore the results are configured
That way ndscan provides a clear initialisation order:
- parameters are always initialised first
- then
get_default_analysesis called - then results are collected
Alternatives
A few options I can think of:
- don't allow analyses to be configured based on parameter values (a real shame because I've found it adds a lot of value in certain circumstances)
- move the code which configures the analyses based on parameter values to somewhere other than
get_default_analyses(e.g. afinalise_analysesmethod) which is called before the schema are finalized for broadcast. I really don't like this approach because it ends up with "partially constructed objects". I find that dataflow is vastly simpler to understand when constructing objects (which includes configuring them) is all done in one place. Separating these steps just introduces footguns and makes code harder to read.
Background
Some notes to self:
TopLevelRunnernever actually callsinit_paramsthere is an undocumented assumption that this will have been called previously, for example byFragmentScanExperimentwhich callsinit_paramsand then constructs aTopLevelRunnerin prepare- during
buildTopLevelRunnercollects result channels and assigns sinks, then callsget_default_analyses TopLevelRunnercallsbroadcast_metadataat the beginning of run (although the schema is frozen during build so changes between build and run result in the applet receiving an incorrect schema although this isn't flagged anywhere)
As a side note: one thing I really struggle with in this part of the code is how spread out schema creation is. There are bits of it all over the place. This makes it really hard to follow the serialization process and reason about things like "when is the last time I can mutate this object and know that changes will be received by the applet?". It would make a huge improvement in ease of following the code if there were a single serialization step where this was all done in one go.