LOKI: dynamic rear-detector geometry from f144 carriage stream#846
Open
SimonHeybrock wants to merge 14 commits intomainfrom
Open
LOKI: dynamic rear-detector geometry from f144 carriage stream#846SimonHeybrock wants to merge 14 commits intomainfrom
SimonHeybrock wants to merge 14 commits intomainfrom
Conversation
SimonHeybrock
force-pushed
the
worktree-dynamic-detector-geometry
branch
from
6ff9d31 to
22ecee5
Compare
Wire LOKI's f144 detector_carriage stream into the rear-bank detector view workflow via a NeXusTransformationChain override provider, plus the dev-mode slider config. Two non-obvious fixes uncovered during interactive testing: - StreamProcessorWorkflow now accepts `initial_context` and triggers a one-shot `set_context` at construction. essreduce caches the parent-of-dynamic layer (e.g. `Projector`) on context updates; without priming, providers downstream of a context key are recomputed on every chunk. With LOKI's cylindrical pixel noise (unseeded RNG in essreduce.position_noise_for_cylindrical_pixel) this caused the random noise replicas to be re-rolled per accumulate, producing tiny coordinate drift and DatasetError on the EternalAccumulator. - LOKI link override uses the full HDF5 path `/entry/instrument/detector_carriage/value`. The transformation chain is keyed by absolute path, not by short alias. Carriage moves still wedge the workflow until clear() per the plan (deferred to #828); this is the documented behaviour and recovery via the manual UI reset works.
GeometricProjector now surfaces any 0-d entries from the projection DataGroup (e.g. `z` for `xy_plane`, `r` for `cylinder_mantle_z`) as scalar coords on every ScreenBinnedEvents batch. Scipp propagates these through hist/sum/slicing, so all detector-view outputs (cumulative, current, counts_total, counts_in_toa_range, ...) now carry a `z` coord that encodes the sample-to-detector distance. This serves two purposes: - It's a projection-independent "has the geometry changed?" signal for the EternalAccumulator. Previously the guard depended on the per-pixel perspective scaling `t = zplane / z` leaking into the xy bin edges, which is fragile: a future change to `project_xy` (e.g. a parallel projection) could silently break it. An explicit scalar coord makes the contract robust. - It is a useful piece of metadata on its own right, readable from the dashboard to confirm the published image reflects the current rear detector carriage position.
Makes call sites self-documenting and gives the heterogeneous (aux stream name, NeXus link path) pair a named home.
…Value* The "link" terminology was non-standard (NeXus calls these "transformations") and the "override" framing implied fixing a wrong value, when in fact this connects a static chain entry to its live value source. Renames: - DetectorTransformLinkName → TransformName - DetectorTransformLinkLogValue → TransformValueLog - DetectorTransformLinkOverride → TransformValue - LinkOverride (factory config) → TransformValueStream - factory parameter link_overrides → transform_value_streams - get_transformation_chain_with_override → ..._with_value - detector_transform_link_override_from_nxlog → transform_value_from_log The new names are also generic enough to be reused for monitors driven by the same f144 streams.
Collapse the spec-side `DetectorCarriageAuxSources` and the factory-side `transform_value_streams` mapping into a single per-instrument constant shared by both. The spec's `render(job_id)` now consults `source_name` and only routes the f144 carriage stream to the rear LOKI bank rather than to all 9 banks. - `TransformValueStream` moves from `detector_view/factory.py` to `detector_view/types.py` so spec and factory can both reference it without pulling factory-level imports. - `DetectorROIAuxSources` gains an optional `dynamic_transforms` mapping; it advertises each unique global aux stream in `inputs` (so the dashboard schema is complete) and routes the entry only to jobs whose `source_name` is in the mapping, un-prefixed. `DetectorCarriageAuxSources` is deleted. - `DetectorViewFactory.transform_value_streams` is renamed `dynamic_transforms` for symmetry with the spec. - LOKI defines `LOKI_DYNAMIC_TRANSFORMS` once in `loki/specs.py` and imports it from `loki/factories.py`, removing the magic-string coupling between the two files. The dashboard's aux-input widget reads `aux_sources.inputs` globally and is purely informational, so this change requires no UI changes. Cosmetic side-effect: the carriage input field still renders on all 9 LOKI banks; hiding it on irrelevant sources would need source-aware `inputs` and is deferred.
SimonHeybrock
force-pushed
the
worktree-dynamic-detector-geometry
branch
from
41d09df to
fc29a6e
Compare
Reuse essreduce's extraction so future changes to how the chain is pulled from the NeXusComponent flow through automatically. Deepcopy only on the mutating branch to avoid leaking changes into the cached component; the pass-through branch matches upstream's aliasing.
Move insertion of get_transformation_chain_with_value and transform_value_from_log out of create_base_workflow into a new add_dynamic_transform helper, called from the factory only when a source actually has a TransformValueStream configured. Instruments without dynamic geometry now get an unchanged graph and skip the extra scheduler dispatch. Drops the empty-name / log-is-None sentinels together with the dummy sc.scalar(0.0) default. TransformValue becomes Optional; the only remaining fallback is the genuine startup case where the f144 log has not yet received a sample, which now returns None. This also avoids the 0.0 default clashing with real stream values arriving later.
- DetectorROIAuxSources: build the full inputs dict before super().__init__ instead of mutating AuxSources._inputs afterwards. - get_transformation_chain_with_value: raise a clear KeyError listing available entries when the configured transform name is missing; early return on the pass-through branch. - TransformValue.name is now typed as TransformName; drops the str(name) cast in transform_value_from_log. TransformValueStream gains slots=True for consistency with TransformValue. - projectors_test: rewrite the "non-scalar entries are not stamped" case to add a real 1-d entry to the coords DataGroup and assert it does not appear on the output. Previous assertion was vacuous.
transform_value_from_log previously returned None and get_transformation_chain_with_value treated that as "use the baked-in value from the reference geometry file". That file's value may be stale or invalid, and producing an output from a wrong geometry is worse than producing no output at all — our system is designed to report such errors. Make transform_value_from_log raise ValueError on an empty log and drop the Optional from get_transformation_chain_with_value. Tests updated accordingly.
Before ``set_context`` is called for the first time, sciline passes ``None`` for the ``TransformValueLog`` parameter, which triggered ``AttributeError: 'NoneType' object has no attribute 'sizes'`` in ``transform_value_from_log``. Accept ``None`` in the signature and fold it into the same "no samples yet" ValueError branch as the empty-log case.
Sciline resolves providers by the declared type key, so annotating
the provider parameter as ``TransformValueLog | None`` would break
the graph: context_keys registers ``TransformValueLog``, and a union
is a different key. Instead, fold the None into the alias itself
(``NewType('TransformValueLog', sc.DataArray | None)``) so the key
the provider sees and the key sciline registers stay identical.
The LOKI rear-bank workflow now refuses to produce output until the f144 detector_carriage stream has delivered a value (fail fast rather than fall back to the reference file's baked-in, possibly-invalid geometry). Publish a carriage log message before the first events in test_can_configure_and_stop_detector_workflow[loki] so the service test exercises the steady-state path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Wires LOKI's
detector_carriagef144 position stream into the rear-bank detector-view workflow so the published image tracks the carriage position as it moves along the beam. Implements the plan indocs/developer/plans/dynamic-detector-geometry-loki-carriage.md.The seam is generic: any instrument can opt in by passing
dynamic_transforms={source: TransformValueStream(...)}toDetectorViewFactoryand usingDetectorROIAuxSources(dynamic_transforms=...)in the workflow spec.Fixes #845.
How it works
add_dynamic_transforminserts two providers into the workflow for sources that opt in: one that extracts the latest sample from the NXlogset_contextstream into aTransformValue, and one that replaces essreduce'sget_transformation_chainwith a variant that deepcopies the chain and injects the live value. If the f144 stream has not yet produced a sample, the provider raises: the reference geometry file's baked-in value is intentionally never used, because it may be stale or invalid and a wrong result is worse than no result. Workflows without dynamic geometry keep the unchanged upstream provider.DetectorROIAuxSourcesnow advertises the configured f144 streams alongside the ROI streams. These streams are physical properties of the instrument (not job-specific), sorender()routes them un-prefixed and only to the jobs whosesource_nameactually consumes them.loki_detector_0) maps thedetector_carriageaux stream onto the NeXus entry/entry/instrument/detector_carriage/value. The mapping lives in a singleLOKI_DYNAMIC_TRANSFORMSdict shared between the spec (for routing) and the factory (for graph wiring).--devslider viaconfigs/log_producer_loki.jsondrives the stream interactively.Known limitation (deferred to #828)
A carriage move mid-run raises on the next
accumulate(different bin edges / differentz) and wedges the workflow until a manual UI reset or nextrun_start. This is expected and documented. The natural follow-up is auto-reset keyed on a change in the scalarzcoord now that the signal is explicit.Test plan
TransformValueextraction, missing-entry error, and scalar-coord stamping (including a negative case for non-scalar entries)zpresent on every output, carriage move correctly wedges the accumulator