Design review: reference semantics, construction API, and implementation strategy #3
d-chambers
started this conversation in
Ideas
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
This summarizes a design review of the inventory proposal against DASCore's existing architecture (Patch/Spool immutability, coordinate management, directory indexing). Straightforward documentation fixes are in #2; this discussion covers the substantive design recommendations and the open questions that came out of the review. The mock API at the end shows the intended end-to-end user experience.
Overall assessment
The core model is sound: the StationXML-parallel layering, the "inventory stays external, patches carry only a
data_source_id" rule, and the four independent interval tracks onOpticalPathall held up against the three worked examples (simple trench, NORFOX, tunnel). The recommendations below are about reference semantics, construction/modification ergonomics, and implementation strategy — not the model's shape.R1: References are IDs; objects are accepted at construction and auto-registered
Shared assets (interrogators, cables, enclosures) must not be embedded inline in the tree. With immutable models, inline copies go stale: updating a cable in
resourceswould leave old copies inside everyFiberSegmentthat embedded it, and value-basedreplace()can't disambiguate identical copies.Proposed semantics:
containerandinterrogatorare typedCable | str(etc.) for construction convenience, but validation normalizes to a resource ID and auto-registers the object intoInventory.resources.inventory.yamlfiles diff cleanly.inventory.resolve(id)); patch methods resolve internally so most users never touch this.The trade-off: a detached
Acquisitioncan't answeracq.interrogator.modelwithout the inventory in hand. This mirrors ObsPy's response-attachment model and is the price of one-edit updates. This matters more now that concurrent routes are separateFiberArrays sharing instruments and cables (e.g., NORFOX's five arms referencing the same three interrogators).R2: Modification is addressed verbs, not tree surgery
Hand-rebuilding nested tuples (
fiber_array.optical_paths[:-1] + (closed, new)) should never be the documented workflow. Three layers:register_fiber_break(...),add_path_revision(...),end_acquisition(...)— each encapsulating "close the current revision, open the new one."inventory.update(address, **changes)where the address is adata_source_idprefix or a resource ID..new()and a strictreplace(old, new)that raises unless the match is unique.R3 (open question): columnar tracks instead of per-interval objects
Geometry, coupling, and annotation tracks are currently tuples of small objects positioned partly by implicit order, where one wrong
optical_lengthsilently shifts everything downstream. But these objects are homogeneous table rows — and the field notes they come from are literally tables. The proposal: each of these three tracks becomes one table-like object per track with explicit distance columns (Geometry.from_table(df),Coupling.from_table(df), ...), whileoptical_componentsstays a tuple of typed objects (heterogeneous types, rich per-type fields, resource links).Benefits: the authoring format, YAML storage format, and compute format become the same columnar structure; explicit distances fix the silent-shift failure;
validate()checks per-track tiling rules (components/geometry/coupling tile[0, L], annotations may overlap).Cost: a table row can't carry a rare rich one-off field the way an object could — overflow goes to an annotation or an
extracolumn. This changes the data model, not just the API, so it needs a decision before spec changes.R4: YAML/dicts are the primary authoring surface
Every nested field should accept a plain dict (native pydantic coercion), making Python construction just the YAML structure written inline — no imports of ~12 leaf classes. The documented workflow becomes: write
inventory.yaml(plus CSVs that are the field notes), load, validate. Typed constructors remain for validation and autocomplete but move to an "advanced" section. Under this, the simple example collapses from ~120 lines of constructors to a ~40-line YAML file anddc.Inventory.from_yaml(...).R5: Patch/spool surface stays small, explicit, and lazy
data_source_ids; it never eagerly enriches patches at instantiation (that would put geometry interpolation in every patch-load loop).unselectverb.on_unmatched="drop"opt-in).on_conflict="raise"available.R6: Hard deployment physics fits existing primitives
Checking the tunnel example against the model, no new object types are needed — but two semantics need defining:
geometry_typeprojection rules:linear→ interpolate;point/coiled(e.g., a buried clump-sensor loop) → all channels in the interval get the constant coordinate.FiberSegment(e.g.fiber_distance_factor) for strain/gauge-length interpretation. The geometry track remains the sole authority for coordinates.The governing principle: every new physical situation should be answered by "which track does this go on," not "what new class do we need."
R7: Implementation strategy — immutable tree + compiled tables
DascoreBaseModel(frozen pydantic,.new(), equality, serialization for free).data_source_id+ time → acquisition/path, enabling whole-index joins for spools) and per-path compiled tracks (sorted numpy(start, stop, payload)arrays; projection isnp.searchsorted+np.interp, O(N log M) per patch).OpticalPathobjects are the same Python objects across inventory versions, so per-path caches survive revisions..patch_attrs.ymldirectory sidecar needs its mtime treated like a data-file mtime for index invalidation, plus documented precedence over file-native attrs.Mock API
End-to-end user story under the recommendations above.
1. Author the inventory as YAML + field-note CSVs
2. Load, validate, use with a spool
3. Enrich a patch explicitly (the only hot-path cost, paid on request)
4. Record field events with timeline verbs
5. Typed construction remains for programmatic/advanced use
Open questions
distancemapping is folded in or stays separate)..patch_attrs.yml) precedence and recursion rules.data_source_idcode parts should carry FDSN-style length/charset restrictions for tooling compatibility.This review was drafted with Claude Code working against the proposal site and the DASCore codebase.
Beta Was this translation helpful? Give feedback.
All reactions