diff --git a/METADATA_STORAGE_V3.md b/METADATA_STORAGE_V3.md index 5f399673f..e891c80b3 100644 --- a/METADATA_STORAGE_V3.md +++ b/METADATA_STORAGE_V3.md @@ -3,8 +3,11 @@ Status: **Rust SDK landed; Python wrappers landed (snapshot model); `include=` + long-format sidecar dfs landed (Wave 1.5); per-kind `*_df` consolidation + `kind: Literal[...]` typing landed (Wave 2, -PR #847); Series accessors and two-mode Attached wrappers deferred -to follow-up PRs** +PR #847); `NamedFunction` SoA migration landed (PR #848 — same +snapshot-model shape as `DecisionVariable` / `Constraint`). +`Series[ID -> Object]` collection accessors are dropped from the +plan (rationale below). Two-mode Attached wrappers remain deferred +to a follow-up PR.** This proposal is a **prerequisite** for `SPECIAL_CONSTRAINTS_V3.md` (PR #841). The proto-schema redesign in #841 cannot be finalized without first deciding @@ -59,10 +62,19 @@ here. The implementation shipped in three waves: type checkers catch typos at edit time; runtime continues to validate against the same set and raise `ValueError` on unknown values for callers without a type checker. -- **Future (deferred):** `Series[ID -> Object]` collection - accessors, the two-mode Standalone / Attached wrappers with - `Py` back-references and write-through metadata setters, - and the `NamedFunction` SoA migration. +- **Future (deferred):** the two-mode Standalone / Attached + wrappers with `Py` back-references and write-through + metadata setters. +- **Dropped:** the `Series[ID -> Object]` collection accessors. + Their original draw was hosting Attached wrappers with bulk + pandas indexing on top; with `*_df` (wide via `include=`, long + via the sidecars) covering bulk analysis from the SoA store + directly and `instance.constraints[id]` covering single-id + access, a Series layer of snapshot wrappers in between would + duplicate the dict accessor without adding a real capability. + The two-mode wrapper work, if it lands, exposes write-through + via the existing `instance.constraints[id]` getter — Series is + not a prerequisite. Sections below mark each item with **(landed)** or **(deferred)** so it is clear which parts are live in v3 alpha and which are still proposal. @@ -80,9 +92,9 @@ hand: metadata replicated as columns next to the type-specific data. We want one **canonical storage** in Rust and well-defined **derived views** -on top of it. The user-visible surfaces (the PyO3 wrapper objects, the new -`Series` collection accessors, the `*_df` methods) all stay — but they -all read from the same SoA store rather than each carrying its own copy. +on top of it. The user-visible surfaces (the PyO3 wrapper objects and +the `*_df` methods) all stay — but they all read from the same SoA +store rather than each carrying its own copy. The duplication shows up in memory accounting (`logical_memory.rs` reports per-row `Option`/`Vec`/`FnvHashMap` headers under @@ -144,9 +156,13 @@ SoA store; the behavior they implement is unchanged. "Wrapper objects with back-reference" below is the long-term target but is **not yet implemented** — the snapshot model is what ships in v3 alpha. -- `instance.constraints`, `decision_variables`, `*_constraints` are - still `dict` / `list` of wrapper objects. The proposed - `pandas.Series[ID -> Object]` shape is **deferred** to a follow-up PR. +- `instance.constraints`, `decision_variables`, `*_constraints` stay + `dict` / `list` of wrapper objects. The earlier proposal to migrate + them to `pandas.Series[ID -> Object]` is **dropped** — bulk analysis + goes through `*_df` (wide via `include=`, long via the sidecars), + which read directly from the SoA store; `dict[id]` still covers + single-id wrapper access. A Series of snapshot wrappers in between + duplicates the dict accessor without adding a real capability. - `*_df` accessors are now methods (not `#[getter]` properties), each with an `include=` parameter that gates the metadata / parameters / removed-reason column families. Default @@ -261,27 +277,15 @@ Why these levels: and `ParametricInstance` already own `BTreeMap` directly. We just add a sibling field. -#### NamedFunction **(deferred — separate PR)** +#### NamedFunction **(landed — PR #848)** -`NamedFunction` currently inlines its metadata as plain fields on the -struct rather than via a `metadata: ConstraintMetadata`-style substruct, -so the v1 → v3 alpha cutover did not include it. The shape today: +`NamedFunction` followed the same SoA migration path as +`DecisionVariable` / `Constraint`, just shipped as a separate PR +because the structural shape was distinct enough that bundling both +diffs would have made the constraint / variable refactor harder to +review. -```rust -pub struct NamedFunction { - pub id: NamedFunctionID, - pub function: Function, - pub name: Option, - pub subscripts: Vec, - pub parameters: FnvHashMap, - pub description: Option, -} -// EvaluatedNamedFunction / SampledNamedFunction follow the same shape. -``` - -For consistency with the rest of the SoA migration, the metadata fields -should move off the per-element struct onto a sibling store on the -enclosing collection — same pattern as `VariableMetadataStore`: +Shape after PR #848: ```rust pub struct NamedFunctionMetadataStore { @@ -300,32 +304,28 @@ pub struct NamedFunction { pub struct Instance { named_functions: BTreeMap, - named_function_metadata: NamedFunctionMetadataStore, // new + named_function_metadata: NamedFunctionMetadataStore, // … } -// ParametricInstance, Solution, SampleSet get the same sibling field +// ParametricInstance, Solution, SampleSet got the same sibling field // next to their named_functions map. ``` The store, the per-field getters, and the parse / serialize drain -pattern are mechanical — they mirror `VariableMetadataStore` -verbatim, with `VariableID` swapped for `NamedFunctionID` and the -provenance field omitted. Pythonside snapshot wrappers -(`NamedFunction`, `EvaluatedNamedFunction`, `SampledNamedFunction`) -gain a second tuple slot for the snapshot, exactly like the -constraint and decision-variable wrappers in PR #843. Pandas -rendering reuses the existing `WithMetadata<'a, T, -NamedFunctionMetadata>` plumbing. - -Scope-wise this is a separate PR from the constraint / variable -migration in #843. The shape of `NamedFunction` is structurally -distinct (no per-element `metadata` substruct to peel off, no -generic `*Collection` to put a store inside, named functions -have no `Created → Evaluated → Sampled` lifecycle), and bundling -both migrations into one diff would have made the constraint / -variable refactor harder to review. The store and per-field -helpers, however, are already in scope of this proposal so the -follow-up has a concrete spec to land against. +pattern mirror `VariableMetadataStore` verbatim, with `VariableID` +swapped for `NamedFunctionID` and the provenance field omitted. +Python-side snapshot wrappers (`NamedFunction`, +`EvaluatedNamedFunction`, `SampledNamedFunction`) gained a second +tuple slot for the snapshot, exactly like the constraint and +decision-variable wrappers in PR #843. Pandas rendering went through +`WithMetadata<'a, T, NamedFunctionMetadata>` to reuse the existing +plumbing. `Instance::evaluate` / `evaluate_samples` thread the +`NamedFunctionMetadataStore` clone onto the produced +`Solution` / `SampleSet`. Element-level `to_bytes` / `from_bytes` +on `NamedFunction` / `EvaluatedNamedFunction` / `SampledNamedFunction` +were removed (same rationale as PR #845: they predate the SoA split +and a per-element wire round-trip can no longer carry the host-stored +metadata). ### Per-object struct changes @@ -489,10 +489,8 @@ The boundaries currently work per-element and need to move to per-collection: - **`pyo3-stub-gen`**: every renamed / removed / added Python method below needs the `gen_stub_pymethods` decorator and the corresponding `ommx.v1.__init__.py` regen via `task python:stubgen`. The stores are - not exposed to Python directly; they surface via wrapper getters, - Series accessors, and DataFrames. For the `Series[ID -> Object]` - return signatures, hand-written stub overrides cover whatever the - derive doesn't emit automatically. + not exposed to Python directly; they surface via wrapper getters and + DataFrames. - **`Evaluate` / Sampled construction paths**: `EvaluatedCollection` and `SampledCollection` currently carry no metadata (`rust/ommx/src/constraint_type.rs`). To realize @@ -506,11 +504,12 @@ The boundaries currently work per-element and need to move to per-collection: ## Python SDK design -The shipping v3 alpha currently exposes only the wrapper-object surface -described under **Snapshot wrappers (landed)** below. The remaining -sections describe the long-term target shape and are explicitly -**deferred** to follow-up PRs; their structure is preserved here so the -Series / `include=` / sidecar work has a concrete spec to land against. +The shipping v3 alpha exposes the snapshot wrapper surface +described under **Snapshot wrappers (landed)** below, plus the +`*_df` family with `include=` / `kind=` / `removed=` covering bulk +analysis. The remaining section on two-mode wrappers describes the +long-term target shape for write-through metadata mutation and is +explicitly **deferred** to follow-up PRs. ### Snapshot wrappers (landed) @@ -568,14 +567,15 @@ or use the Rust SoA setters. The two-mode design described next is the long-term replacement for the snapshot model — it would let `c.set_name("x")` write through to the instance's SoA store. Adopting it requires `Py` back-references -on every wrapper and is gated behind the deferred Series accessor work. +on every wrapper and is independent of the dropped Series accessor +plan; write-through plugs into the existing `instance.constraints[id]` +getter. -### Layered views over the Rust SoA store **(partially landed)** +### Layered views over the Rust SoA store **(landed)** `include=` and the long-format sidecar dfs landed in PR #846; the `kind=Literal[...]` consolidation of per-kind `*_constraints_df` -landed in PR #847 (Wave 2). Only the Series accessor remains -deferred. +landed in PR #847 (Wave 2). ``` Rust SoA store (canonical) @@ -590,20 +590,21 @@ deferred. │ ConstraintCollection (Py wrapper) │ └─────┬─────────────────────────────┘ │ - ┌────────────┼─────────────────────┐ - ▼ ▼ ▼ - Series Constraint object constraints_df(kind=..., include=...) / - (per-id (with .name / constraint_metadata_df(kind=...) / - wrapper .subscripts / … constraint_parameters_df(kind=...) / - handles) back-referenced constraint_provenance_df(kind=...) / - to the store) constraint_removed_reasons_df(kind=...) - (bulk-built from the SoA via column-wise - builders; kind typed Literal[...]) + ┌─────┴──────────────────────┐ + ▼ ▼ + instance.constraints[id] constraints_df(kind=..., include=...) / + (snapshot Constraint object constraint_metadata_df(kind=...) / + with .name / .subscripts / constraint_parameters_df(kind=...) / + …; v3-alpha snapshot model, constraint_provenance_df(kind=...) / + future two-mode wrappers constraint_removed_reasons_df(kind=...) + write through to the store) (bulk-built from the SoA via column-wise + builders; kind typed Literal[...]) ``` -Wrapper objects, Series, and DataFrames are three views over the same -store. The wrapper getters and the DataFrame columns produce the same -values for the same ID; the difference is bulk vs. per-id ergonomics. +Wrapper objects and DataFrames are two views over the same store. +The wrapper getters and the DataFrame columns produce the same +values for the same ID; the difference is per-id wrapper ergonomics +vs. bulk DataFrame ergonomics. ### Wrapper objects with back-reference **(deferred)** @@ -643,8 +644,8 @@ Standalone wrapper is **transitioned in place**: its `inner` flips to `c.add_name(...)`, etc. read / write the SoA store. Calling `add_constraint(c)` a second time on an already-Attached wrapper raises `ValueError("constraint already inserted")` rather than -silently re-inserting. Series-derived wrappers (`s.loc[id]`) are -also Attached, sharing the `Py` of the parent. +silently re-inserting. Wrappers obtained from `instance.constraints[id]` +are also Attached, sharing the `Py` of the parent. Two Attached wrappers that point at the same id observe the same state: a write through one (`a.name = "x"`) is visible through any @@ -662,9 +663,9 @@ print(c.name) # "balance" — read from staging bag attached = instance.add_constraint(c) print(attached.name) # "balance" — read from SoA via back-reference -# Series access — Attached wrappers -s = instance.constraints -print(s.loc[5].name) # back-reference lookup; same value as the metadata df +# Single-id access — Attached wrapper +print(instance.constraints[5].name) +# back-reference lookup; same value as the metadata df # Mutation — write-through to SoA attached.name = "demand_balance" @@ -687,52 +688,43 @@ reference can't dangle. Open semantic question: The simple rule: a wrapper's `id` stays in either the active or removed map for the lifetime of the parent `Instance`, so getters never panic. -### Series-based collection accessors **(deferred)** - -```python -s = instance.constraints # pandas.Series[ConstraintID -> Constraint] -s.loc[5] # individual Constraint object (Attached) -s.loc[5].equality # type-intrinsic getter -s.loc[5].name # metadata getter via back-reference - -list(s.index) # all constraint IDs -for cid, c in s.items(): ... # iteration - -# decision variables and the other constraint kinds get the same treatment -instance.decision_variables # Series[VariableID -> DecisionVariable] -instance.indicator_constraints # Series[IndicatorConstraintID -> IndicatorConstraint] -instance.one_hot_constraints # Series[OneHotConstraintID -> OneHotConstraint] -instance.sos1_constraints # Series[Sos1ConstraintID -> Sos1Constraint] - -solution.constraints # Series[ConstraintID -> EvaluatedConstraint] -sample_set.constraints # Series[ConstraintID -> SampledConstraint] - -# ParametricInstance follows the same surface as Instance: Series -# accessors for variables / constraints / special constraints, the -# corresponding constraints_df / constraint_metadata_df / etc. -# family with the same `include=` parameter, and back-referenced -# wrappers for individual elements. The (unrelated) parametric -# `parameters` field stays on its own dedicated accessor. -parametric_instance.constraints # Series[ConstraintID -> Constraint] -parametric_instance.decision_variables # Series[VariableID -> DecisionVariable] -``` - -The Series carries Attached wrapper objects (object dtype). Per-element -efficiency is the same as the old `dict[ID, Constraint]`. Indexing -operations users get for free vs. dict: `.loc`, `.iloc`, boolean -indexing, `.items()`, `.index`. Operations users lose vs. dict: - -- **`s.values()` is NOT a method**; pandas `Series.values` is a - property returning a numpy array. Existing `.values()` calls break - loudly. Migration: `s.tolist()`, `list(s)`, or `for c in s:`. -- **`s[id]` works for an integer id** because Series allows index-by- - label lookup with `[]`, but `.loc[id]` is the explicit form. - Documentation should prefer `.loc[id]` to avoid the - position-vs-label ambiguity. -- **`s.apply(lambda c: c.equality)` is an attractive nuisance**: it - iterates Python-side and rebuilds the equality column row-by-row. - The right answer is `instance.constraints_df()["equality"]`, which - is bulk-built from the SoA. Document this; do not enforce. +### Series-based collection accessors **(dropped)** + +The earlier waves of this proposal positioned +`instance.constraints: pandas.Series[ConstraintID -> Constraint]` +(and the parallel accessors on `decision_variables`, the special +constraint kinds, and the Solution / SampleSet hosts) as a +third surface alongside the wrapper objects and the `*_df` methods. +After Wave 1.5 + Wave 2 the proposal no longer pays for itself: + +- **Bulk analysis already reads from the SoA store directly.** + `instance.constraints_df(kind=..., include=...)` (wide) and + `constraint_metadata_df(kind=...)` / `constraint_parameters_df` / + `constraint_provenance_df` / `constraint_removed_reasons_df` + (long-format sidecars) are the right place to do bulk filtering, + joins, and aggregation. They are column-wise builders over the + SoA; a Series of snapshot wrappers would force users back into + per-row Python-side loops, which is the antipattern we want to + steer them away from. +- **Per-id access already has a clean shape.** + `instance.constraints[id]` returns a wrapper today and would keep + doing so once two-mode wrappers land (the Attached mode plugs + into the same getter). `s.loc[id]` would be a second spelling + of the same thing. +- **The bulk indexing primitives Series adds are not load-bearing + here.** `.loc` / `.iloc` / boolean indexing on a Series of object + dtype iterates in Python and rebuilds the result row-by-row; + the natural answer in this codebase is "ask the DataFrame," not + "iterate the Series." +- **The `.values()` migration cost is real.** `pandas.Series.values` + is a property, not a method; making `instance.constraints` a + Series breaks every existing `.values()` call loudly with no + upside on the bulk-analysis path. + +So `instance.constraints` etc. stay as `dict[ID, Wrapper]` / `list` +in v3. The two-mode wrapper work is independent of this decision — +write-through metadata mutation plugs into `instance.constraints[id]` +directly, without a Series detour. ### `*_df` methods → derived views with `include` and `kind=` **(Wave 1.5 + Wave 2)** @@ -900,10 +892,11 @@ a column-level concern, not a row-level one. `*_df` (with `include=`) is what users call when they want a wide table for analysis; the long-format sidecars are what they call when -they want tidy data for joins or aggregation; the Series is what they -call when they want individual wrapper objects; the wrapper getters -are what they call when they already hold one wrapper. Four surfaces, -one canonical store. +they want tidy data for joins or aggregation; the dict accessor +(`instance.constraints[id]`) is what they call when they want an +individual wrapper object; the wrapper getters are what they call +when they already hold one wrapper. Three surfaces, one canonical +store. ### Avoiding cross-ID-space joins @@ -972,7 +965,7 @@ ExtensionArray is a meaningful pandas integration and a maintenance burden disproportionate to the failure mode it prevents. The qualified index name + `include=` covering the common wide case are sufficient. -### `ToPandasEntry` restructuring **(landed for snapshot model + sidecars; Series deferred)** +### `ToPandasEntry` restructuring **(landed)** `python/ommx/src/pandas.rs` previously had ~16 `ToPandasEntry` impls that read `self.metadata.X` directly off each element. The shipping @@ -1000,9 +993,11 @@ shape is: read directly from the SoA stores (via `store.name(id)` / `.parameters(id)` / `.provenance(id)`) and don't go through `ToPandasEntry` at all — they're bulk column-wise builders. -- **Series accessors (deferred).** The current accessors still return - `dict` / `list`; the `pandas.Series[ID -> Object]` shape with - Attached wrappers is part of the deferred wave. +- **Collection accessors stay `dict` / `list`.** The earlier + proposal to migrate `instance.constraints` etc. to + `pandas.Series[ID -> Object]` is dropped — see the + "Series-based collection accessors **(dropped)**" section above + for the rationale. ### `subscripts` / `provenance` representation @@ -1129,23 +1124,55 @@ shape is: `*_df` accessor returns a uniform `pandas.DataFrame`, so overload-style typing has no return-type variation to express. +### Landed in NamedFunction SoA migration (PR #848) + +- `NamedFunction`, `EvaluatedNamedFunction`, and + `SampledNamedFunction` lose their inline `name` / `subscripts` / + `parameters` / `description` fields. Metadata moves to a + `NamedFunctionMetadataStore` SoA store kept as a sibling field on + `Instance`, `ParametricInstance`, `Solution`, and `SampleSet`. + `Instance::evaluate` / `evaluate_samples` thread the store onto + the produced `Solution` / `SampleSet` so per-element metadata + reads through the same channel as decision-variable and + constraint metadata. +- Element-level `to_bytes` / `from_bytes` methods on the three + per-element types are removed (same rationale as PR #845: a + per-element wire round-trip can no longer carry the host-stored + metadata, so the methods became misleading rather than safe). +- Python wrappers (`NamedFunction`, `EvaluatedNamedFunction`, + `SampledNamedFunction`) gain the snapshot tuple slot and read + metadata from it, mirroring `DecisionVariable` / `Constraint` + post-PR #843. `Instance.from_components(named_functions=...)` + drains each wrapper's snapshot into the host's + `NamedFunctionMetadataStore`. +- `*_df` rendering for named functions goes through the existing + `WithMetadata<'a, T, NamedFunctionMetadata>` plumbing: the host + pre-snapshots the SoA store and zips metadata in alongside each + element before handing the iterator to `entries_to_dataframe`. +- Rust SDK call sites that previously read + `named_function.name` / `.subscripts` / `.description` + (e.g. `Instance::named_function_names`, + `Instance::get_named_function_by_name`, + `Solution::extract_named_functions`, + `SampleSet::extract_all_named_functions`) now read from the + per-host `named_function_metadata` store. Behavior is unchanged. + ### Deferred to follow-up wave -- `instance.constraints`, `decision_variables`, `*_constraints` change - type from `dict` / `list` to `pandas.Series`. Most usage (`s.loc[id]`, - iteration, `.items()`, `.index`) keeps working. Specific breakage: - - `s.values()` (method call) → `s.tolist()` or `list(s)`. - - List-positional reliance on the old `decision_variables: list[…]` - ordering breaks; index by `VariableID` instead. - Wrapper-object metadata setters become write-through to the SoA - store via the Standalone / Attached two-mode design. -- `NamedFunction` / `EvaluatedNamedFunction` / - `SampledNamedFunction` lose their inline `name` / `subscripts` / - `parameters` / `description` fields; metadata moves to a - `NamedFunctionMetadataStore` sibling field on `Instance` / - `ParametricInstance` / `Solution` / `SampleSet`. Same shape as - the constraint / variable migration in this PR but lands as a - separate PR. + store via the Standalone / Attached two-mode design. Plugs into + the existing `instance.constraints[id]` getter; no Series detour. + +### Dropped from the plan + +- `instance.constraints`, `decision_variables`, `*_constraints` were + going to change type from `dict` / `list` to `pandas.Series`. With + `*_df` covering bulk analysis directly off the SoA store and + `dict[id]` covering single-id wrapper access, a Series of snapshot + wrappers in between would not pull its weight (and would have + broken `.values()` callers loudly with no upside). See the + "Series-based collection accessors **(dropped)**" section for the + full rationale. A new section in `PYTHON_SDK_MIGRATION_GUIDE.md` will cover the Python side in detail once the deferred wave lands. @@ -1269,10 +1296,11 @@ traceability with earlier review comments. no code-level mitigation in this proposal.** The wrapper holds a refcounted handle to the Instance; there's no cycle (wrapper → Instance, Instance → store, no back-pointer from store to - wrapper), but heavy use of Series can keep an Instance alive - longer than expected. Users who care drop the Series. Whether the - pattern needs revisiting (e.g. weak-handle variant of the wrapper) - is a question to take up at implementation time, not before. + wrapper), but holding a long-lived collection of Attached wrappers + can keep an Instance alive longer than expected. Users who care + drop their references to those wrappers. Whether the pattern + needs revisiting (e.g. weak-handle variant of the wrapper) is a + question to take up at implementation time, not before. ## Follow-ups (post-#843, post-#846) @@ -1284,15 +1312,14 @@ traceability with earlier review comments. `*_metadata_mut`). These three methods should be narrowed to `pub(crate)` (or smaller) in a follow-up so the only way to break the collection's invariants is from inside the crate. -- **`NamedFunction` SoA migration.** Track the - `NamedFunctionMetadataStore` work described under - "NamedFunction (deferred — separate PR)" above. -- **Python Series + Attached two-mode wrappers.** Wave 2 of the - Python surface — `instance.constraints` returns a - `pandas.Series[ID -> Object]`, wrappers gain a `Py` - back-reference and write-through metadata setters. Blocked on - the snapshot-vs-attached decision; the `Py` lifetime - question is documented in resolved decision #8. +- **Attached two-mode wrappers.** Wrappers gain a `Py` + back-reference and write-through metadata setters via the + Standalone / Attached two-mode design. Plugs into the existing + `instance.constraints[id]` getter (the Series-based collection + accessor proposal that originally bundled this work has been + dropped — see "Series-based collection accessors **(dropped)**" + for rationale). The `Py` lifetime question is + documented in resolved decision #8. ## Open questions diff --git a/docs/api/api_reference.json b/docs/api/api_reference.json index d339545fd..da62a623c 100644 --- a/docs/api/api_reference.json +++ b/docs/api/api_reference.json @@ -4932,7 +4932,7 @@ { "kind": "Class", "name": "EvaluatedNamedFunction", - "doc": "EvaluatedNamedFunction wrapper for Python", + "doc": "EvaluatedNamedFunction wrapper for Python.\n\nHolds the Rust `EvaluatedNamedFunction` plus an owned snapshot of its\nmetadata. See `NamedFunction` for the snapshot-model rationale.", "bases": [], "methods": [ { @@ -10699,7 +10699,7 @@ { "kind": "Class", "name": "NamedFunction", - "doc": "NamedFunction wrapper for Python", + "doc": "NamedFunction wrapper for Python.\n\nHolds the Rust `NamedFunction` plus an owned snapshot of its metadata\n(`name` / `subscripts` / `parameters` / `description`). The metadata\nstore lives at the host (`Instance` / `Solution` / `SampleSet`) level;\nwhen a wrapper is created via a host accessor, the host snapshots its\nstore into the second tuple slot. Mutations on a wrapper do NOT\npropagate back to the host — re-insert via `Instance.from_components`\n(or equivalent) to apply changes. Same shape as `Constraint` /\n`DecisionVariable` after PR #843.", "bases": [], "methods": [ { @@ -18454,7 +18454,7 @@ { "kind": "Class", "name": "SampledNamedFunction", - "doc": "", + "doc": "SampledNamedFunction wrapper for Python.\n\nHolds the Rust `SampledNamedFunction` plus an owned snapshot of its\nmetadata. See `NamedFunction` for the snapshot-model rationale.", "bases": [], "methods": [ { diff --git a/python/ommx/ommx/_ommx_rust/__init__.pyi b/python/ommx/ommx/_ommx_rust/__init__.pyi index 22d5d9684..8a9bd13fc 100644 --- a/python/ommx/ommx/_ommx_rust/__init__.pyi +++ b/python/ommx/ommx/_ommx_rust/__init__.pyi @@ -986,7 +986,10 @@ class EvaluatedDecisionVariable: @typing.final class EvaluatedNamedFunction: r""" - EvaluatedNamedFunction wrapper for Python + EvaluatedNamedFunction wrapper for Python. + + Holds the Rust `EvaluatedNamedFunction` plus an owned snapshot of its + metadata. See `NamedFunction` for the snapshot-model rationale. """ @property def id(self) -> builtins.int: ... @@ -2958,7 +2961,16 @@ class Linear: @typing.final class NamedFunction: r""" - NamedFunction wrapper for Python + NamedFunction wrapper for Python. + + Holds the Rust `NamedFunction` plus an owned snapshot of its metadata + (`name` / `subscripts` / `parameters` / `description`). The metadata + store lives at the host (`Instance` / `Solution` / `SampleSet`) level; + when a wrapper is created via a host accessor, the host snapshots its + store into the second tuple slot. Mutations on a wrapper do NOT + propagate back to the host — re-insert via `Instance.from_components` + (or equivalent) to apply changes. Same shape as `Constraint` / + `DecisionVariable` after PR #843. """ @property def id(self) -> builtins.int: ... @@ -4296,6 +4308,12 @@ class SampledDecisionVariable: @typing.final class SampledNamedFunction: + r""" + SampledNamedFunction wrapper for Python. + + Holds the Rust `SampledNamedFunction` plus an owned snapshot of its + metadata. See `NamedFunction` for the snapshot-model rationale. + """ @property def id(self) -> builtins.int: r""" diff --git a/python/ommx/src/evaluated_named_function.rs b/python/ommx/src/evaluated_named_function.rs index 308ae6261..6c61f5b2f 100644 --- a/python/ommx/src/evaluated_named_function.rs +++ b/python/ommx/src/evaluated_named_function.rs @@ -1,11 +1,17 @@ use pyo3::{prelude::*, Bound}; use std::collections::{HashMap, HashSet}; -/// EvaluatedNamedFunction wrapper for Python +/// EvaluatedNamedFunction wrapper for Python. +/// +/// Holds the Rust `EvaluatedNamedFunction` plus an owned snapshot of its +/// metadata. See `NamedFunction` for the snapshot-model rationale. #[pyo3_stub_gen::derive::gen_stub_pyclass] #[pyclass] #[derive(Clone)] -pub struct EvaluatedNamedFunction(pub ommx::EvaluatedNamedFunction); +pub struct EvaluatedNamedFunction( + pub ommx::EvaluatedNamedFunction, + pub ommx::NamedFunctionMetadata, +); #[pyo3_stub_gen::derive::gen_stub_pymethods] #[pymethods] @@ -22,22 +28,22 @@ impl EvaluatedNamedFunction { #[getter] pub fn name(&self) -> Option { - self.0.name.clone() + self.1.name.clone() } #[getter] pub fn subscripts(&self) -> Vec { - self.0.subscripts.clone() + self.1.subscripts.clone() } #[getter] pub fn parameters(&self) -> HashMap { - self.0.parameters.clone().into_iter().collect() + self.1.parameters.clone().into_iter().collect() } #[getter] pub fn description(&self) -> Option { - self.0.description.clone() + self.1.description.clone() } #[getter] diff --git a/python/ommx/src/instance.rs b/python/ommx/src/instance.rs index 842433b8c..788099eb2 100644 --- a/python/ommx/src/instance.rs +++ b/python/ommx/src/instance.rs @@ -172,10 +172,15 @@ impl Instance { builder = builder.sos1_constraints(rust_sos1_constraints); } + let mut named_function_metadata_pairs: Vec<( + ommx::NamedFunctionID, + ommx::NamedFunctionMetadata, + )> = Vec::new(); if let Some(nfs) = named_functions { let mut rust_named_functions = BTreeMap::new(); for nf in nfs { let id = nf.0.id; + named_function_metadata_pairs.push((id, nf.1)); if rust_named_functions.insert(id, nf.0).is_some() { anyhow::bail!("Duplicate named function ID: {}", id.into_inner()); } @@ -201,6 +206,10 @@ impl Instance { for (id, m) in indicator_metadata_pairs { indicator_meta.insert(id, m); } + let nf_meta = inner.named_function_metadata_mut(); + for (id, m) in named_function_metadata_pairs { + nf_meta.insert(id, m); + } Ok(Self { inner, @@ -458,10 +467,13 @@ impl Instance { /// List of all named functions in the instance sorted by their IDs. #[getter] pub fn named_functions(&self) -> Vec { + let metadata = self.inner.named_function_metadata(); self.inner .named_functions() - .values() - .map(|named_function| NamedFunction(named_function.clone())) + .iter() + .map(|(id, named_function)| { + NamedFunction(named_function.clone(), metadata.collect_for(*id)) + }) .collect() } @@ -1825,7 +1837,21 @@ impl Instance { include: Option>, ) -> PyResult> { let flags = crate::pandas::IncludeFlags::from_optional(include)?; - entries_to_dataframe(py, self.inner.named_functions().values(), "id", flags) + let nf_meta_store = self.inner.named_function_metadata().clone(); + let nf_meta_view: Vec<(ommx::NamedFunctionMetadata, &ommx::NamedFunction)> = self + .inner + .named_functions() + .iter() + .map(|(id, nf)| (nf_meta_store.collect_for(*id), nf)) + .collect(); + entries_to_dataframe( + py, + nf_meta_view + .iter() + .map(|(m, nf)| crate::pandas::WithMetadata::new(*nf, m)), + "id", + flags, + ) } /// Constraint metadata DataFrame (id-indexed wide format). @@ -2127,10 +2153,16 @@ impl Instance { /// Get a specific named function by ID pub fn get_named_function_by_id(&self, named_function_id: u64) -> PyResult { + let id = NamedFunctionID::from(named_function_id); self.inner .named_functions() - .get(&NamedFunctionID::from(named_function_id)) - .map(|named_function| NamedFunction(named_function.clone())) + .get(&id) + .map(|named_function| { + NamedFunction( + named_function.clone(), + self.inner.named_function_metadata().collect_for(id), + ) + }) .ok_or_else(|| { PyKeyError::new_err(format!( "Named function with ID {named_function_id} not found" diff --git a/python/ommx/src/named_function.rs b/python/ommx/src/named_function.rs index 17be8c74d..9b1cf7416 100644 --- a/python/ommx/src/named_function.rs +++ b/python/ommx/src/named_function.rs @@ -3,11 +3,20 @@ use ommx::{Evaluate, NamedFunctionID}; use pyo3::{prelude::*, Bound, PyAny}; use std::collections::HashMap; -/// NamedFunction wrapper for Python +/// NamedFunction wrapper for Python. +/// +/// Holds the Rust `NamedFunction` plus an owned snapshot of its metadata +/// (`name` / `subscripts` / `parameters` / `description`). The metadata +/// store lives at the host (`Instance` / `Solution` / `SampleSet`) level; +/// when a wrapper is created via a host accessor, the host snapshots its +/// store into the second tuple slot. Mutations on a wrapper do NOT +/// propagate back to the host — re-insert via `Instance.from_components` +/// (or equivalent) to apply changes. Same shape as `Constraint` / +/// `DecisionVariable` after PR #843. #[pyo3_stub_gen::derive::gen_stub_pyclass] #[pyclass] #[derive(Clone)] -pub struct NamedFunction(pub ommx::NamedFunction); +pub struct NamedFunction(pub ommx::NamedFunction, pub ommx::NamedFunctionMetadata); #[pyo3_stub_gen::derive::gen_stub_pymethods] #[pymethods] @@ -38,13 +47,15 @@ impl NamedFunction { let named_function = ommx::NamedFunction { id: named_function_id, function: rust_function, + }; + let metadata = ommx::NamedFunctionMetadata { name, subscripts, parameters: parameters.into_iter().collect(), description, }; - Ok(Self(named_function)) + Ok(Self(named_function, metadata)) } #[getter] @@ -59,22 +70,22 @@ impl NamedFunction { #[getter] pub fn name(&self) -> Option { - self.0.name.clone() + self.1.name.clone() } #[getter] pub fn subscripts(&self) -> Vec { - self.0.subscripts.clone() + self.1.subscripts.clone() } #[getter] pub fn parameters(&self) -> HashMap { - self.0.parameters.clone().into_iter().collect() + self.1.parameters.clone().into_iter().collect() } #[getter] pub fn description(&self) -> Option { - self.0.description.clone() + self.1.description.clone() } /// Evaluate the named function with the given state. @@ -96,7 +107,9 @@ impl NamedFunction { .0 .evaluate(&state.0, atol) .map_err(|e| pyo3::exceptions::PyValueError::new_err(e.to_string()))?; - Ok(EvaluatedNamedFunction(evaluated)) + // Per-element evaluate doesn't see the host's metadata store, so the + // metadata snapshot from the source `NamedFunction` carries over. + Ok(EvaluatedNamedFunction(evaluated, self.1.clone())) } /// Partially evaluate the named function with the given state. diff --git a/python/ommx/src/pandas.rs b/python/ommx/src/pandas.rs index 5f040681b..efaf4a731 100644 --- a/python/ommx/src/pandas.rs +++ b/python/ommx/src/pandas.rs @@ -1262,20 +1262,22 @@ impl<'m> ToPandasEntry } } -impl ToPandasEntry for ommx::NamedFunction { +impl<'m> ToPandasEntry for WithMetadata<'m, &ommx::NamedFunction, ommx::NamedFunctionMetadata> { fn to_pandas_entry<'py>(&self, py: Python<'py>) -> PyResult> { + let nf = self.item; + let m = self.metadata; let dict = PyDict::new(py); - dict.set_item("id", self.id.into_inner())?; - set_function_type(&dict, &self.function)?; - dict.set_item("function", crate::Function(self.function.clone()))?; - set_used_ids(&dict, &self.function.required_ids())?; + dict.set_item("id", nf.id.into_inner())?; + set_function_type(&dict, &nf.function)?; + dict.set_item("function", crate::Function(nf.function.clone()))?; + set_used_ids(&dict, &nf.function.required_ids())?; set_metadata( &dict, - self.name.as_deref(), - &self.subscripts, - self.description.as_deref(), + m.name.as_deref(), + &m.subscripts, + m.description.as_deref(), )?; - set_parameter_columns(&dict, &self.parameters)?; + set_parameter_columns(&dict, &m.parameters)?; Ok(dict) } } @@ -1353,19 +1355,23 @@ impl<'m> ToPandasEntry } } -impl ToPandasEntry for ommx::EvaluatedNamedFunction { +impl<'m> ToPandasEntry + for WithMetadata<'m, &ommx::EvaluatedNamedFunction, ommx::NamedFunctionMetadata> +{ fn to_pandas_entry<'py>(&self, py: Python<'py>) -> PyResult> { + let enf = self.item; + let m = self.metadata; let dict = PyDict::new(py); - dict.set_item("id", self.id.into_inner())?; - dict.set_item("value", self.evaluated_value())?; - set_used_ids(&dict, self.used_decision_variable_ids())?; + dict.set_item("id", enf.id.into_inner())?; + dict.set_item("value", enf.evaluated_value())?; + set_used_ids(&dict, enf.used_decision_variable_ids())?; set_metadata( &dict, - self.name.as_deref(), - &self.subscripts, - self.description.as_deref(), + m.name.as_deref(), + &m.subscripts, + m.description.as_deref(), )?; - set_parameter_columns(&dict, &self.parameters)?; + set_parameter_columns(&dict, &m.parameters)?; Ok(dict) } } @@ -1442,20 +1448,27 @@ impl<'a, 'm> ToPandasEntry } } -impl<'a> ToPandasEntry for WithSampleIds<'a, &'a ommx::SampledNamedFunction> { +impl<'a, 'm> ToPandasEntry + for WithMetadata< + 'm, + WithSampleIds<'a, &'a ommx::SampledNamedFunction>, + ommx::NamedFunctionMetadata, + > +{ fn to_pandas_entry<'py>(&self, py: Python<'py>) -> PyResult> { - let nf = self.item; + let nf = self.item.item; + let m = self.metadata; let dict = PyDict::new(py); dict.set_item("id", nf.id().into_inner())?; set_used_ids(&dict, nf.used_decision_variable_ids())?; set_metadata( &dict, - nf.name.as_deref(), - &nf.subscripts, - nf.description.as_deref(), + m.name.as_deref(), + &m.subscripts, + m.description.as_deref(), )?; - set_parameter_columns(&dict, &nf.parameters)?; - for &sample_id in self.sample_ids { + set_parameter_columns(&dict, &m.parameters)?; + for &sample_id in self.item.sample_ids { let value = nf.evaluated_values().get(sample_id).copied(); dict.set_item(sample_id.into_inner(), value)?; } diff --git a/python/ommx/src/parametric_instance.rs b/python/ommx/src/parametric_instance.rs index 062b6a628..8a1f397b7 100644 --- a/python/ommx/src/parametric_instance.rs +++ b/python/ommx/src/parametric_instance.rs @@ -90,10 +90,15 @@ impl ParametricInstance { .constraints(rust_constraints) .parameters(rust_parameters); + let mut named_function_metadata_pairs: Vec<( + ommx::NamedFunctionID, + ommx::NamedFunctionMetadata, + )> = Vec::new(); if let Some(nfs) = named_functions { let mut rust_named_functions = BTreeMap::new(); for nf in nfs { let id = nf.0.id; + named_function_metadata_pairs.push((id, nf.1)); if rust_named_functions.insert(id, nf.0).is_some() { anyhow::bail!("Duplicate named function ID: {}", id.into_inner()); } @@ -114,6 +119,10 @@ impl ParametricInstance { for (id, m) in constraint_metadata_pairs { constraint_meta.insert(id, m); } + let nf_meta = inner.named_function_metadata_mut(); + for (id, m) in named_function_metadata_pairs { + nf_meta.insert(id, m); + } Ok(Self { inner, @@ -200,10 +209,11 @@ impl ParametricInstance { #[getter] pub fn named_functions(&self) -> Vec { + let metadata = self.inner.named_function_metadata(); self.inner .named_functions() - .values() - .map(|nf| NamedFunction(nf.clone())) + .iter() + .map(|(id, nf)| NamedFunction(nf.clone(), metadata.collect_for(*id))) .collect() } @@ -287,10 +297,16 @@ impl ParametricInstance { /// Get a specific named function by ID pub fn get_named_function_by_id(&self, named_function_id: u64) -> PyResult { + let id = NamedFunctionID::from(named_function_id); self.inner .named_functions() - .get(&NamedFunctionID::from(named_function_id)) - .map(|nf| NamedFunction(nf.clone())) + .get(&id) + .map(|nf| { + NamedFunction( + nf.clone(), + self.inner.named_function_metadata().collect_for(id), + ) + }) .ok_or_else(|| { PyKeyError::new_err(format!( "Named function with ID {named_function_id} not found" @@ -418,7 +434,21 @@ impl ParametricInstance { include: Option>, ) -> PyResult> { let flags = crate::pandas::IncludeFlags::from_optional(include)?; - entries_to_dataframe(py, self.inner.named_functions().values(), "id", flags) + let nf_meta_store = self.inner.named_function_metadata().clone(); + let nf_meta_view: Vec<(ommx::NamedFunctionMetadata, &ommx::NamedFunction)> = self + .inner + .named_functions() + .iter() + .map(|(id, nf)| (nf_meta_store.collect_for(*id), nf)) + .collect(); + entries_to_dataframe( + py, + nf_meta_view + .iter() + .map(|(m, nf)| crate::pandas::WithMetadata::new(*nf, m)), + "id", + flags, + ) } /// DataFrame of parameters diff --git a/python/ommx/src/sample_set.rs b/python/ommx/src/sample_set.rs index bf568c55d..d31fd770f 100644 --- a/python/ommx/src/sample_set.rs +++ b/python/ommx/src/sample_set.rs @@ -237,10 +237,11 @@ impl SampleSet { /// Get named functions for compatibility with existing Python code #[getter] pub fn named_functions(&self) -> Vec { + let metadata = self.inner.named_function_metadata(); self.inner .named_functions() - .values() - .map(|nf| crate::SampledNamedFunction(nf.clone())) + .iter() + .map(|(id, nf)| crate::SampledNamedFunction(nf.clone(), metadata.collect_for(*id))) .collect() } @@ -480,7 +481,14 @@ impl SampleSet { self.inner .named_functions() .get(&named_function_id) - .map(|nf| crate::SampledNamedFunction(nf.clone())) + .map(|nf| { + crate::SampledNamedFunction( + nf.clone(), + self.inner + .named_function_metadata() + .collect_for(named_function_id), + ) + }) .ok_or_else(|| { pyo3::exceptions::PyKeyError::new_err(format!( "Unknown named function ID: {named_function_id}" @@ -698,15 +706,24 @@ impl SampleSet { ) -> PyResult> { let flags = crate::pandas::IncludeFlags::from_optional(include)?; let sample_ids = sorted_sample_ids(&self.inner); + let nf_meta_store = self.inner.named_function_metadata().clone(); + let nf_meta_view: Vec<(ommx::NamedFunctionMetadata, &ommx::SampledNamedFunction)> = self + .inner + .named_functions() + .iter() + .map(|(id, nf)| (nf_meta_store.collect_for(*id), nf)) + .collect(); entries_to_dataframe( py, - self.inner - .named_functions() - .values() - .map(|item| WithSampleIds { - item, - sample_ids: &sample_ids, - }), + nf_meta_view.iter().map(|(m, nf)| { + crate::pandas::WithMetadata::new( + WithSampleIds { + item: *nf, + sample_ids: &sample_ids, + }, + m, + ) + }), "id", flags, ) diff --git a/python/ommx/src/sampled_named_function.rs b/python/ommx/src/sampled_named_function.rs index 26c7121c4..f5bef76e4 100644 --- a/python/ommx/src/sampled_named_function.rs +++ b/python/ommx/src/sampled_named_function.rs @@ -1,10 +1,17 @@ use pyo3::{prelude::*, Bound}; use std::collections::{BTreeMap, HashSet}; +/// SampledNamedFunction wrapper for Python. +/// +/// Holds the Rust `SampledNamedFunction` plus an owned snapshot of its +/// metadata. See `NamedFunction` for the snapshot-model rationale. #[pyo3_stub_gen::derive::gen_stub_pyclass] #[pyclass] #[derive(Clone)] -pub struct SampledNamedFunction(pub ommx::SampledNamedFunction); +pub struct SampledNamedFunction( + pub ommx::SampledNamedFunction, + pub ommx::NamedFunctionMetadata, +); #[pyo3_stub_gen::derive::gen_stub_pymethods] #[pymethods] @@ -18,25 +25,25 @@ impl SampledNamedFunction { /// Get the named function name #[getter] pub fn name(&self) -> Option { - self.0.name.clone() + self.1.name.clone() } /// Get the subscripts #[getter] pub fn subscripts(&self) -> Vec { - self.0.subscripts.clone() + self.1.subscripts.clone() } /// Get the description #[getter] pub fn description(&self) -> Option { - self.0.description.clone() + self.1.description.clone() } /// Get the parameters #[getter] pub fn parameters(&self) -> std::collections::HashMap { - self.0 + self.1 .parameters .iter() .map(|(k, v)| (k.clone(), v.clone())) diff --git a/python/ommx/src/solution.rs b/python/ommx/src/solution.rs index 2f019ca4e..3f0327363 100644 --- a/python/ommx/src/solution.rs +++ b/python/ommx/src/solution.rs @@ -170,10 +170,11 @@ impl Solution { /// Get evaluated named functions as a list sorted by ID #[getter] pub fn named_functions(&self) -> Vec { + let metadata = self.inner.named_function_metadata(); self.inner .evaluated_named_functions() - .values() - .map(|nf| crate::EvaluatedNamedFunction(nf.clone())) + .iter() + .map(|(id, nf)| crate::EvaluatedNamedFunction(nf.clone(), metadata.collect_for(*id))) .collect() } @@ -462,7 +463,14 @@ impl Solution { self.inner .evaluated_named_functions() .get(&named_function_id) - .map(|nf| crate::EvaluatedNamedFunction(nf.clone())) + .map(|nf| { + crate::EvaluatedNamedFunction( + nf.clone(), + self.inner + .named_function_metadata() + .collect_for(named_function_id), + ) + }) .ok_or_else(|| { PyKeyError::new_err(format!( "Unknown named function ID: {}", @@ -565,9 +573,18 @@ impl Solution { include: Option>, ) -> PyResult> { let flags = crate::pandas::IncludeFlags::from_optional(include)?; + let nf_meta_store = self.inner.named_function_metadata().clone(); + let nf_meta_view: Vec<(ommx::NamedFunctionMetadata, &ommx::EvaluatedNamedFunction)> = self + .inner + .evaluated_named_functions() + .iter() + .map(|(id, nf)| (nf_meta_store.collect_for(*id), nf)) + .collect(); entries_to_dataframe( py, - self.inner.evaluated_named_functions().values(), + nf_meta_view + .iter() + .map(|(m, nf)| crate::pandas::WithMetadata::new(*nf, m)), "id", flags, ) diff --git a/rust/ommx/src/instance.rs b/rust/ommx/src/instance.rs index 58127221b..cbb2a8fdc 100644 --- a/rust/ommx/src/instance.rs +++ b/rust/ommx/src/instance.rs @@ -155,6 +155,11 @@ pub struct Instance { #[getset(get = "pub")] named_functions: BTreeMap, + /// Per-named-function auxiliary metadata. Sibling field of + /// [`Self::named_functions`]; together they form the canonical + /// named-function storage. + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, + // Optional fields for additional metadata. // These fields are public since arbitrary values can be set without validation. pub parameters: Option, @@ -172,6 +177,18 @@ impl Instance { &mut self.variable_metadata } + /// Access the per-named-function metadata store. + pub fn named_function_metadata(&self) -> &crate::named_function::NamedFunctionMetadataStore { + &self.named_function_metadata + } + + /// Mutable access to the per-named-function metadata store. + pub fn named_function_metadata_mut( + &mut self, + ) -> &mut crate::named_function::NamedFunctionMetadataStore { + &mut self.named_function_metadata + } + /// Active constraints. pub fn constraints(&self) -> &BTreeMap { self.constraint_collection.active() @@ -445,6 +462,11 @@ pub struct ParametricInstance { #[getset(get = "pub")] named_functions: BTreeMap, + /// Per-named-function auxiliary metadata. Sibling field of + /// [`Self::named_functions`]; together they form the canonical + /// named-function storage. + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, + // Optional fields for additional metadata. // These fields are public since arbitrary values can be set without validation. pub description: Option, @@ -461,6 +483,18 @@ impl ParametricInstance { &mut self.variable_metadata } + /// Access the per-named-function metadata store. + pub fn named_function_metadata(&self) -> &crate::named_function::NamedFunctionMetadataStore { + &self.named_function_metadata + } + + /// Mutable access to the per-named-function metadata store. + pub fn named_function_metadata_mut( + &mut self, + ) -> &mut crate::named_function::NamedFunctionMetadataStore { + &mut self.named_function_metadata + } + /// Active constraints. pub fn constraints(&self) -> &BTreeMap { self.constraint_collection.active() diff --git a/rust/ommx/src/instance/arbitrary.rs b/rust/ommx/src/instance/arbitrary.rs index 5cd4cc03e..3dff83135 100644 --- a/rust/ommx/src/instance/arbitrary.rs +++ b/rust/ommx/src/instance/arbitrary.rs @@ -234,6 +234,7 @@ impl Arbitrary for Instance { Default::default(), ), named_functions, + named_function_metadata: Default::default(), sense, decision_variables, variable_metadata: Default::default(), diff --git a/rust/ommx/src/instance/builder.rs b/rust/ommx/src/instance/builder.rs index 8f5a1e8f1..973727fb7 100644 --- a/rust/ommx/src/instance/builder.rs +++ b/rust/ommx/src/instance/builder.rs @@ -403,6 +403,7 @@ impl InstanceBuilder { BTreeMap::new(), ), named_functions: self.named_functions, + named_function_metadata: Default::default(), decision_variable_dependency: self.decision_variable_dependency, parameters: self.parameters, description: self.description, @@ -770,10 +771,6 @@ mod tests { let named_function = NamedFunction { id: NamedFunctionID::from(1), function: Function::Zero, - name: Some("f".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; let err = Instance::builder() @@ -805,10 +802,6 @@ mod tests { let named_function = NamedFunction { id: NamedFunctionID::from(1), function: Function::from(linear!(999) + coeff!(1.0)), - name: Some("f".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; let err = Instance::builder() diff --git a/rust/ommx/src/instance/convert.rs b/rust/ommx/src/instance/convert.rs index a50456a37..a05371069 100644 --- a/rust/ommx/src/instance/convert.rs +++ b/rust/ommx/src/instance/convert.rs @@ -50,6 +50,7 @@ impl From for ParametricInstance { decision_variable_dependency, description, named_functions, + named_function_metadata, .. }: Instance, ) -> Self { @@ -66,6 +67,7 @@ impl From for ParametricInstance { decision_variable_dependency, description, named_functions, + named_function_metadata, } } } @@ -130,6 +132,7 @@ impl ParametricInstance { one_hot_constraint_collection: self.one_hot_constraint_collection, sos1_constraint_collection: self.sos1_constraint_collection, named_functions, + named_function_metadata: self.named_function_metadata, decision_variable_dependency: self.decision_variable_dependency, parameters: Some(parameters), description: self.description, diff --git a/rust/ommx/src/instance/evaluate.rs b/rust/ommx/src/instance/evaluate.rs index 2cc3fe646..b1e0efc02 100644 --- a/rust/ommx/src/instance/evaluate.rs +++ b/rust/ommx/src/instance/evaluate.rs @@ -77,6 +77,7 @@ impl Evaluate for Instance { .evaluated_named_functions(evaluated_named_functions) .decision_variables(decision_variables) .variable_metadata(self.variable_metadata.clone()) + .named_function_metadata(self.named_function_metadata.clone()) .sense(sense) .build_unchecked()? }; @@ -146,6 +147,7 @@ impl Evaluate for Instance { .one_hot_constraints_collection(sampled_one_hot_constraints) .sos1_constraints_collection(sampled_sos1_constraints) .named_functions(named_functions) + .named_function_metadata(self.named_function_metadata.clone()) .sense(self.sense) .build()?) } @@ -393,10 +395,6 @@ mod tests { let named_function = NamedFunction { id: NamedFunctionID::from(1), function: Function::from(linear!(2) + linear!(3) + linear!(4) + linear!(5)), - name: Some("f".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; let named_functions = btreemap! { diff --git a/rust/ommx/src/instance/logical_memory.rs b/rust/ommx/src/instance/logical_memory.rs index 13981e908..9a461266d 100644 --- a/rust/ommx/src/instance/logical_memory.rs +++ b/rust/ommx/src/instance/logical_memory.rs @@ -173,6 +173,10 @@ mod tests { Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.provenance;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;removed_indicator_constraints;BTreeMap[stack] 24 + Instance.named_function_metadata;NamedFunctionMetadataStore.description;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.name;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.parameters;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.named_functions;BTreeMap[stack] 24 Instance.objective;Zero 40 Instance.one_hot_constraint_collection;metadata;ConstraintMetadataStore.description;FnvHashMap[stack] 32 @@ -242,6 +246,10 @@ mod tests { Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.provenance;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;removed_indicator_constraints;BTreeMap[stack] 24 + Instance.named_function_metadata;NamedFunctionMetadataStore.description;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.name;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.parameters;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.named_functions;BTreeMap[stack] 24 Instance.objective;Linear;PolynomialBase.terms 80 Instance.one_hot_constraint_collection;metadata;ConstraintMetadataStore.description;FnvHashMap[stack] 32 @@ -324,6 +332,10 @@ mod tests { Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.provenance;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;removed_indicator_constraints;BTreeMap[stack] 24 + Instance.named_function_metadata;NamedFunctionMetadataStore.description;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.name;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.parameters;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.named_functions;BTreeMap[stack] 24 Instance.objective;Linear;PolynomialBase.terms 80 Instance.one_hot_constraint_collection;metadata;ConstraintMetadataStore.description;FnvHashMap[stack] 32 @@ -400,6 +412,10 @@ mod tests { Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.provenance;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;removed_indicator_constraints;BTreeMap[stack] 24 + Instance.named_function_metadata;NamedFunctionMetadataStore.description;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.name;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.parameters;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.named_functions;BTreeMap[stack] 24 Instance.objective;Zero 40 Instance.one_hot_constraint_collection;metadata;ConstraintMetadataStore.description;FnvHashMap[stack] 32 @@ -487,6 +503,10 @@ mod tests { Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.provenance;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;metadata;ConstraintMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.indicator_constraint_collection;removed_indicator_constraints;BTreeMap[stack] 24 + Instance.named_function_metadata;NamedFunctionMetadataStore.description;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.name;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.parameters;FnvHashMap[stack] 32 + Instance.named_function_metadata;NamedFunctionMetadataStore.subscripts;FnvHashMap[stack] 32 Instance.named_functions;BTreeMap[stack] 24 Instance.objective;Zero 40 Instance.one_hot_constraint_collection;metadata;ConstraintMetadataStore.description;FnvHashMap[stack] 32 diff --git a/rust/ommx/src/instance/named_function.rs b/rust/ommx/src/instance/named_function.rs index bcc1f336d..b1db2d8e2 100644 --- a/rust/ommx/src/instance/named_function.rs +++ b/rust/ommx/src/instance/named_function.rs @@ -9,8 +9,8 @@ impl Instance { /// Named functions without names are not included. pub fn named_function_names(&self) -> std::collections::BTreeSet { self.named_functions - .values() - .filter_map(|var| var.name.clone()) + .keys() + .filter_map(|id| self.named_function_metadata().name(*id).map(str::to_owned)) .collect() } @@ -39,9 +39,14 @@ impl Instance { name: &str, subscripts: Vec, ) -> Option<&NamedFunction> { - self.named_functions - .values() - .find(|var| var.name.as_deref() == Some(name) && var.subscripts == subscripts) + self.named_functions.iter().find_map(|(id, nf)| { + let store = self.named_function_metadata(); + if store.name(*id) == Some(name) && store.subscripts(*id) == subscripts.as_slice() { + Some(nf) + } else { + None + } + }) } /// Returns the next available NamedFunctionID. @@ -75,15 +80,15 @@ impl Instance { } let id = self.next_named_function_id(); - let named_function = NamedFunction { - id, - function, + let named_function = NamedFunction { id, function }; + self.named_functions.insert(id, named_function); + let metadata = crate::named_function::NamedFunctionMetadata { name, subscripts, parameters, description, }; - self.named_functions.insert(id, named_function); + self.named_function_metadata_mut().insert(id, metadata); Ok(self.named_functions.get_mut(&id).unwrap()) } } @@ -162,8 +167,12 @@ mod tests { // Lookup by correct name and subscripts let found = instance.get_named_function_by_name("f", vec![1, 2]); assert!(found.is_some()); - assert_eq!(found.unwrap().name, Some("f".to_string())); - assert_eq!(found.unwrap().subscripts, vec![1, 2]); + let found_id = found.unwrap().id; + assert_eq!(instance.named_function_metadata().name(found_id), Some("f")); + assert_eq!( + instance.named_function_metadata().subscripts(found_id), + &[1, 2] + ); // Wrong subscripts → None let not_found = instance.get_named_function_by_name("f", vec![3]); @@ -228,10 +237,12 @@ mod tests { ) .unwrap(); - assert_eq!(nf.id, NamedFunctionID::from(0)); - assert_eq!(nf.name, Some("obj".to_string())); - assert_eq!(nf.subscripts, vec![0]); - assert_eq!(nf.description, Some("test function".to_string())); + let nf_id = nf.id; + assert_eq!(nf_id, NamedFunctionID::from(0)); + let store = instance.named_function_metadata(); + assert_eq!(store.name(nf_id), Some("obj")); + assert_eq!(store.subscripts(nf_id), &[0]); + assert_eq!(store.description(nf_id), Some("test function")); } #[test] diff --git a/rust/ommx/src/instance/parametric_builder.rs b/rust/ommx/src/instance/parametric_builder.rs index 18ccc90dd..d1c53b274 100644 --- a/rust/ommx/src/instance/parametric_builder.rs +++ b/rust/ommx/src/instance/parametric_builder.rs @@ -284,6 +284,7 @@ impl ParametricInstanceBuilder { one_hot_constraint_collection: Default::default(), sos1_constraint_collection: Default::default(), named_functions: self.named_functions, + named_function_metadata: Default::default(), decision_variable_dependency: self.decision_variable_dependency, description: self.description, }) @@ -370,10 +371,6 @@ mod tests { let named_function = NamedFunction { id: NamedFunctionID::from(1), function: Function::Zero, - name: Some("f".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; let err = ParametricInstance::builder() @@ -406,10 +403,6 @@ mod tests { let named_function = NamedFunction { id: NamedFunctionID::from(1), function: Function::from(linear!(999) + coeff!(1.0)), - name: Some("f".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; let err = ParametricInstance::builder() @@ -443,10 +436,6 @@ mod tests { let named_function = NamedFunction { id: NamedFunctionID::from(1), function: Function::from(linear!(1) + linear!(2)), // uses both decision var and param - name: Some("f".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; let instance = ParametricInstance::builder() diff --git a/rust/ommx/src/instance/parse.rs b/rust/ommx/src/instance/parse.rs index d89af42b8..2d2977e0a 100644 --- a/rust/ommx/src/instance/parse.rs +++ b/rust/ommx/src/instance/parse.rs @@ -193,7 +193,10 @@ impl Parse for v1::Instance { removed_constraints.insert(id, (c, reason)); } - let named_functions = self + let (named_functions, named_function_metadata): ( + BTreeMap, + crate::named_function::NamedFunctionMetadataStore, + ) = self .named_functions .parse_as(&(), message, "named_functions")?; @@ -255,6 +258,7 @@ impl Parse for v1::Instance { parameters: self.parameters, description: self.description, named_functions, + named_function_metadata, }) } } @@ -283,10 +287,14 @@ impl From for v1::Instance { .into_iter() .map(|(id, c)| constraint_to_v1(id, c, constraint_metadata.remove(id))) .collect(); + let mut named_function_metadata = value.named_function_metadata; let named_functions = value .named_functions - .into_values() - .map(|nf| nf.into()) + .into_iter() + .map(|(id, nf)| { + let metadata = named_function_metadata.remove(id); + crate::named_function::parse::named_function_to_v1(nf, metadata) + }) .collect(); let removed_constraints = removed .into_iter() @@ -419,7 +427,10 @@ impl Parse for v1::ParametricInstance { removed_constraints.insert(id, (c, reason)); } - let named_functions = self + let (named_functions, named_function_metadata): ( + BTreeMap, + crate::named_function::NamedFunctionMetadataStore, + ) = self .named_functions .parse_as(&(), message, "named_functions")?; @@ -479,6 +490,7 @@ impl Parse for v1::ParametricInstance { ), sos1_constraint_collection: ConstraintCollection::new(sos1_active, BTreeMap::new()), named_functions, + named_function_metadata, decision_variable_dependency, description: self.description, }) @@ -500,6 +512,7 @@ impl From for v1::ParametricInstance { decision_variable_dependency, description, named_functions, + named_function_metadata, }: ParametricInstance, ) -> Self { // Special constraint types do not have a v1 proto representation yet. @@ -537,6 +550,14 @@ impl From for v1::ParametricInstance { .into_iter() .map(|(id, (c, r))| removed_constraint_to_v1(id, c, constraint_metadata.remove(id), r)) .collect(); + let mut named_function_metadata = named_function_metadata; + let v1_named_functions = named_functions + .into_iter() + .map(|(id, nf)| { + let metadata = named_function_metadata.remove(id); + crate::named_function::parse::named_function_to_v1(nf, metadata) + }) + .collect(); Self { description, sense: v1::instance::Sense::from(sense) as i32, @@ -544,7 +565,7 @@ impl From for v1::ParametricInstance { decision_variables: v1_decision_variables, parameters: parameters.into_values().collect(), constraints: v1_constraints, - named_functions: named_functions.into_values().map(|nf| nf.into()).collect(), + named_functions: v1_named_functions, removed_constraints: v1_removed_constraints, decision_variable_dependency: decision_variable_dependency .into_iter() @@ -1150,4 +1171,51 @@ mod tests { Some("demand-balance row"), ); } + + /// Regression: `From for v1::Instance` and the matching + /// `Parse for v1::Instance` must drain / re-attach the + /// `named_function_metadata` SoA store across a bytes round-trip, + /// the same way constraint and variable metadata do. + #[test] + fn test_instance_roundtrip_preserves_named_function_metadata() { + use crate::{ + coeff, linear, DecisionVariable, Function, NamedFunctionID, Sense, VariableID, + }; + + let var_id = VariableID::from(1); + let nf_id = NamedFunctionID::from(0); + + let mut instance = Instance::builder() + .sense(Sense::Minimize) + .objective(Function::from(linear!(1))) + .decision_variables(maplit::btreemap! { + var_id => DecisionVariable::binary(var_id), + }) + .constraints(BTreeMap::new()) + .build() + .unwrap(); + + instance + .new_named_function( + Function::from(linear!(1) + coeff!(1.0)), + Some("offset_x".to_string()), + vec![0], + fnv::FnvHashMap::default(), + Some("x plus a constant".to_string()), + ) + .unwrap(); + + let bytes = instance.to_bytes(); + let recovered = Instance::from_bytes(&bytes).unwrap(); + + assert_eq!( + recovered.named_function_metadata().name(nf_id), + Some("offset_x"), + ); + assert_eq!(recovered.named_function_metadata().subscripts(nf_id), &[0]); + assert_eq!( + recovered.named_function_metadata().description(nf_id), + Some("x plus a constant"), + ); + } } diff --git a/rust/ommx/src/instance/penalty.rs b/rust/ommx/src/instance/penalty.rs index 365fe8e8a..0a3400481 100644 --- a/rust/ommx/src/instance/penalty.rs +++ b/rust/ommx/src/instance/penalty.rs @@ -125,6 +125,7 @@ impl Instance { decision_variable_dependency: self.decision_variable_dependency, description: self.description, named_functions: self.named_functions, + named_function_metadata: self.named_function_metadata, }) } @@ -200,6 +201,7 @@ impl Instance { decision_variable_dependency: self.decision_variable_dependency, description: self.description, named_functions: self.named_functions, + named_function_metadata: self.named_function_metadata, }); } @@ -265,6 +267,7 @@ impl Instance { decision_variable_dependency: self.decision_variable_dependency, description: self.description, named_functions: self.named_functions, + named_function_metadata: self.named_function_metadata, }) } } diff --git a/rust/ommx/src/named_function.rs b/rust/ommx/src/named_function.rs index 2df985cb0..7f7c56e84 100644 --- a/rust/ommx/src/named_function.rs +++ b/rust/ommx/src/named_function.rs @@ -1,7 +1,7 @@ mod arbitrary; mod evaluate; -mod parse; -mod serialize; +mod metadata_store; +pub(crate) mod parse; use derive_more::{Deref, From}; use fnv::FnvHashMap; @@ -10,6 +10,7 @@ use getset::*; use crate::logical_memory::{LogicalMemoryProfile, LogicalMemoryVisitor, Path}; use crate::{Function, SampleID, Sampled, VariableIDSet}; pub use arbitrary::*; +pub use metadata_store::NamedFunctionMetadataStore; /// ID for named function #[derive( @@ -68,45 +69,40 @@ impl LogicalMemoryProfile for NamedFunctionID { /// Named function IDs are managed separately from decision variable IDs and constraint IDs, /// so the same ID value can be used across these different namespaces. /// +/// Metadata (`name`, `subscripts`, `parameters`, `description`) is stored in a +/// per-collection [`NamedFunctionMetadataStore`] keyed by [`NamedFunctionID`]; +/// the per-element struct no longer carries it. +/// /// Corresponds to `ommx.v1.NamedFunction`. #[derive(Debug, Clone, PartialEq, LogicalMemoryProfile)] pub struct NamedFunction { pub id: NamedFunctionID, pub function: Function, - pub name: Option, - pub subscripts: Vec, - pub parameters: FnvHashMap, - pub description: Option, } impl std::fmt::Display for NamedFunction { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let name_str = self - .name - .as_ref() - .map(|n| format!("name=\"{n}\"")) - .unwrap_or_else(|| "name=None".to_string()); - - let mut parts = vec![name_str]; - - if !self.subscripts.is_empty() { - parts.push(format!("subscripts={:?}", self.subscripts)); - } - - if !self.parameters.is_empty() { - let params: Vec = self - .parameters - .iter() - .map(|(k, v)| format!("{k}={v}")) - .collect(); - parts.push(format!("parameters={{{}}}", params.join(", "))); - } - - write!(f, "NamedFunction({}, {})", self.function, parts.join(", ")) + write!(f, "NamedFunction(id={}, {})", self.id, self.function) } } +/// Auxiliary metadata for named functions (excluding intrinsic id and function). +/// +/// Sibling type to [`DecisionVariableMetadata`](crate::DecisionVariableMetadata). +/// Stored ID-keyed in [`NamedFunctionMetadataStore`]. +#[derive(Debug, Clone, PartialEq, Default, LogicalMemoryProfile)] +pub struct NamedFunctionMetadata { + pub name: Option, + pub subscripts: Vec, + pub parameters: FnvHashMap, + pub description: Option, +} + /// `ommx.v1.EvaluatedNamedFunction` with validated, typed fields. +/// +/// Metadata moved to a per-collection +/// [`NamedFunctionMetadataStore`] on `Solution`; the struct only carries +/// intrinsic evaluated data. #[derive(Debug, Clone, PartialEq, CopyGetters, Getters)] pub struct EvaluatedNamedFunction { #[getset(get_copy = "pub")] @@ -114,45 +110,24 @@ pub struct EvaluatedNamedFunction { #[getset(get_copy = "pub")] pub evaluated_value: f64, #[getset(get = "pub")] - pub name: Option, - #[getset(get = "pub")] - pub subscripts: Vec, - #[getset(get = "pub")] - pub parameters: FnvHashMap, - #[getset(get = "pub")] - pub description: Option, - #[getset(get = "pub")] used_decision_variable_ids: VariableIDSet, } impl std::fmt::Display for EvaluatedNamedFunction { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let name_str = self - .name - .as_ref() - .map(|n| format!("name=\"{n}\"")) - .unwrap_or_else(|| "name=None".to_string()); - - let mut parts = vec![name_str, format!("value={}", self.evaluated_value)]; - - if !self.subscripts.is_empty() { - parts.push(format!("subscripts={:?}", self.subscripts)); - } - - if !self.parameters.is_empty() { - let params: Vec = self - .parameters - .iter() - .map(|(k, v)| format!("{k}={v}")) - .collect(); - parts.push(format!("parameters={{{}}}", params.join(", "))); - } - - write!(f, "EvaluatedNamedFunction({})", parts.join(", ")) + write!( + f, + "EvaluatedNamedFunction(id={}, value={})", + self.id, self.evaluated_value + ) } } -/// Multiple sample evaluation results with deduplication +/// Multiple sample evaluation results with deduplication. +/// +/// Metadata moved to a per-collection +/// [`NamedFunctionMetadataStore`] on `SampleSet`; the struct only carries +/// intrinsic sampled data. #[derive(Debug, Clone, PartialEq, Getters)] pub struct SampledNamedFunction { #[getset(get = "pub")] @@ -160,44 +135,17 @@ pub struct SampledNamedFunction { #[getset(get = "pub")] evaluated_values: Sampled, #[getset(get = "pub")] - pub name: Option, - #[getset(get = "pub")] - pub subscripts: Vec, - #[getset(get = "pub")] - pub parameters: FnvHashMap, - #[getset(get = "pub")] - pub description: Option, - #[getset(get = "pub")] used_decision_variable_ids: VariableIDSet, } impl std::fmt::Display for SampledNamedFunction { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let name_str = self - .name - .as_ref() - .map(|n| format!("name=\"{n}\"")) - .unwrap_or_else(|| "name=None".to_string()); - - let mut parts = vec![ - name_str, - format!("num_samples={}", self.evaluated_values.num_samples()), - ]; - - if !self.subscripts.is_empty() { - parts.push(format!("subscripts={:?}", self.subscripts)); - } - - if !self.parameters.is_empty() { - let params: Vec = self - .parameters - .iter() - .map(|(k, v)| format!("{k}={v}")) - .collect(); - parts.push(format!("parameters={{{}}}", params.join(", "))); - } - - write!(f, "SampledNamedFunction({})", parts.join(", ")) + write!( + f, + "SampledNamedFunction(id={}, num_samples={})", + self.id, + self.evaluated_values.num_samples() + ) } } @@ -211,10 +159,6 @@ impl SampledNamedFunction { Some(EvaluatedNamedFunction { id: *self.id(), evaluated_value, - name: self.name.clone(), - subscripts: self.subscripts.clone(), - parameters: self.parameters.clone(), - description: self.description.clone(), used_decision_variable_ids: self.used_decision_variable_ids.clone(), }) } diff --git a/rust/ommx/src/named_function/arbitrary.rs b/rust/ommx/src/named_function/arbitrary.rs index 06919e72b..6d0c83461 100644 --- a/rust/ommx/src/named_function/arbitrary.rs +++ b/rust/ommx/src/named_function/arbitrary.rs @@ -13,10 +13,6 @@ impl Arbitrary for NamedFunction { .prop_map(|function| NamedFunction { id: NamedFunctionID(0), // Should be replaced with a unique ID, but cannot be generated here function, - name: None, - subscripts: Vec::new(), - parameters: Default::default(), - description: None, }) .boxed() } diff --git a/rust/ommx/src/named_function/evaluate.rs b/rust/ommx/src/named_function/evaluate.rs index 5bcafe3be..7ec7a1dd3 100644 --- a/rust/ommx/src/named_function/evaluate.rs +++ b/rust/ommx/src/named_function/evaluate.rs @@ -15,10 +15,6 @@ impl Evaluate for NamedFunction { Ok(EvaluatedNamedFunction { id: self.id, evaluated_value, - name: self.name.clone(), - subscripts: self.subscripts.clone(), - parameters: self.parameters.clone(), - description: self.description.clone(), used_decision_variable_ids, }) } @@ -45,10 +41,6 @@ impl Evaluate for NamedFunction { Ok(SampledNamedFunction { id: self.id, evaluated_values, - name: self.name.clone(), - subscripts: self.subscripts.clone(), - parameters: self.parameters.clone(), - description: self.description.clone(), used_decision_variable_ids, }) } @@ -66,10 +58,6 @@ mod tests { let nf = NamedFunction { id: NamedFunctionID::from(1), function: Function::Constant(Coefficient::try_from(42.0).unwrap()), - name: Some("my_func".to_string()), - subscripts: vec![1, 2], - parameters: Default::default(), - description: Some("constant function".to_string()), }; let state = crate::v1::State::default(); @@ -77,9 +65,6 @@ mod tests { assert_eq!(result.id(), NamedFunctionID::from(1)); assert_eq!(result.evaluated_value(), 42.0); - assert_eq!(*result.name(), Some("my_func".to_string())); - assert_eq!(*result.subscripts(), vec![1, 2]); - assert_eq!(*result.description(), Some("constant function".to_string())); assert!(result.used_decision_variable_ids().is_empty()); } @@ -89,10 +74,6 @@ mod tests { let nf = NamedFunction { id: NamedFunctionID::from(2), function: Function::Linear(coeff!(2.0) * linear!(1) + coeff!(3.0) * linear!(2)), - name: Some("linear_func".to_string()), - subscripts: vec![], - parameters: Default::default(), - description: None, }; // x1 = 5.0, x2 = 10.0 => 2*5 + 3*10 = 40.0 @@ -114,10 +95,6 @@ mod tests { let nf = NamedFunction { id: NamedFunctionID::from(3), function: Function::Linear(coeff!(2.0) * linear!(1) + coeff!(3.0) * linear!(2)), - name: None, - subscripts: vec![], - parameters: Default::default(), - description: None, }; let ids = nf.required_ids(); diff --git a/rust/ommx/src/named_function/metadata_store.rs b/rust/ommx/src/named_function/metadata_store.rs new file mode 100644 index 000000000..27157a916 --- /dev/null +++ b/rust/ommx/src/named_function/metadata_store.rs @@ -0,0 +1,195 @@ +use crate::logical_memory::LogicalMemoryProfile; +use crate::named_function::{NamedFunctionID, NamedFunctionMetadata}; +use fnv::FnvHashMap; +use std::sync::OnceLock; + +fn empty_parameters() -> &'static FnvHashMap { + static EMPTY: OnceLock> = OnceLock::new(); + EMPTY.get_or_init(FnvHashMap::default) +} + +/// ID-keyed Struct-of-Arrays storage for named-function metadata. +/// +/// Sibling type to [`VariableMetadataStore`](crate::VariableMetadataStore), +/// living next to the [`Instance`](crate::Instance)'s +/// `named_functions: BTreeMap` map. Same +/// shape as `VariableMetadataStore`: there is no `provenance` field — +/// that is constraint-specific. +#[derive(Debug, Clone, PartialEq, Default, LogicalMemoryProfile)] +pub struct NamedFunctionMetadataStore { + name: FnvHashMap, + subscripts: FnvHashMap>, + parameters: FnvHashMap>, + description: FnvHashMap, +} + +impl NamedFunctionMetadataStore { + pub fn new() -> Self { + Self::default() + } + + // ===== Per-field borrowing getters ===== + + pub fn name(&self, id: NamedFunctionID) -> Option<&str> { + self.name.get(&id).map(String::as_str) + } + + pub fn subscripts(&self, id: NamedFunctionID) -> &[i64] { + self.subscripts.get(&id).map_or(&[], Vec::as_slice) + } + + pub fn parameters(&self, id: NamedFunctionID) -> &FnvHashMap { + self.parameters + .get(&id) + .unwrap_or_else(|| empty_parameters()) + } + + pub fn description(&self, id: NamedFunctionID) -> Option<&str> { + self.description.get(&id).map(String::as_str) + } + + /// Reconstruct the full metadata for a single id as an owned struct. + pub fn collect_for(&self, id: NamedFunctionID) -> NamedFunctionMetadata { + NamedFunctionMetadata { + name: self.name.get(&id).cloned(), + subscripts: self.subscripts.get(&id).cloned().unwrap_or_default(), + parameters: self.parameters.get(&id).cloned().unwrap_or_default(), + description: self.description.get(&id).cloned(), + } + } + + // ===== Setters ===== + + pub fn set_name(&mut self, id: NamedFunctionID, name: impl Into) { + self.name.insert(id, name.into()); + } + + pub fn clear_name(&mut self, id: NamedFunctionID) { + self.name.remove(&id); + } + + pub fn set_subscripts(&mut self, id: NamedFunctionID, s: impl Into>) { + let s = s.into(); + if s.is_empty() { + self.subscripts.remove(&id); + } else { + self.subscripts.insert(id, s); + } + } + + pub fn set_parameter( + &mut self, + id: NamedFunctionID, + key: impl Into, + value: impl Into, + ) { + self.parameters + .entry(id) + .or_default() + .insert(key.into(), value.into()); + } + + pub fn set_parameters(&mut self, id: NamedFunctionID, params: FnvHashMap) { + if params.is_empty() { + self.parameters.remove(&id); + } else { + self.parameters.insert(id, params); + } + } + + pub fn set_description(&mut self, id: NamedFunctionID, desc: impl Into) { + self.description.insert(id, desc.into()); + } + + pub fn clear_description(&mut self, id: NamedFunctionID) { + self.description.remove(&id); + } + + // ===== Bulk owned exchange ===== + + /// Insert metadata for one id, replacing any existing entry. Empty fields + /// are not stored, keeping the maps sparse. + pub fn insert(&mut self, id: NamedFunctionID, metadata: NamedFunctionMetadata) { + let NamedFunctionMetadata { + name, + subscripts, + parameters, + description, + } = metadata; + match name { + Some(n) => self.name.insert(id, n), + None => self.name.remove(&id), + }; + if subscripts.is_empty() { + self.subscripts.remove(&id); + } else { + self.subscripts.insert(id, subscripts); + } + if parameters.is_empty() { + self.parameters.remove(&id); + } else { + self.parameters.insert(id, parameters); + } + match description { + Some(d) => self.description.insert(id, d), + None => self.description.remove(&id), + }; + } + + pub fn remove(&mut self, id: NamedFunctionID) -> NamedFunctionMetadata { + NamedFunctionMetadata { + name: self.name.remove(&id), + subscripts: self.subscripts.remove(&id).unwrap_or_default(), + parameters: self.parameters.remove(&id).unwrap_or_default(), + description: self.description.remove(&id), + } + } + + pub fn contains(&self, id: NamedFunctionID) -> bool { + self.name.contains_key(&id) + || self.subscripts.contains_key(&id) + || self.parameters.contains_key(&id) + || self.description.contains_key(&id) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn empty_store_returns_neutral_values() { + let store = NamedFunctionMetadataStore::new(); + let id = NamedFunctionID::from(1); + assert_eq!(store.name(id), None); + assert!(store.subscripts(id).is_empty()); + assert!(store.parameters(id).is_empty()); + assert_eq!(store.description(id), None); + assert_eq!(store.collect_for(id), NamedFunctionMetadata::default()); + assert!(!store.contains(id)); + } + + #[test] + fn insert_then_collect_round_trip() { + let mut store = NamedFunctionMetadataStore::new(); + let id = NamedFunctionID::from(42); + let mut params = FnvHashMap::default(); + params.insert("k".into(), "v".into()); + let metadata = NamedFunctionMetadata { + name: Some("f".to_string()), + subscripts: vec![0, 1], + parameters: params, + description: Some("d".to_string()), + }; + store.insert(id, metadata.clone()); + assert_eq!(store.collect_for(id), metadata); + } + + #[test] + fn empty_metadata_does_not_create_entries() { + let mut store = NamedFunctionMetadataStore::new(); + let id = NamedFunctionID::from(0); + store.insert(id, NamedFunctionMetadata::default()); + assert!(!store.contains(id)); + } +} diff --git a/rust/ommx/src/named_function/parse.rs b/rust/ommx/src/named_function/parse.rs index 7f8f73805..58f9cef14 100644 --- a/rust/ommx/src/named_function/parse.rs +++ b/rust/ommx/src/named_function/parse.rs @@ -7,148 +7,153 @@ use crate::{ }; use anyhow::Result; +/// Parsed v1 `NamedFunction` together with its drained metadata. +/// +/// Per-element parse no longer attaches metadata to the [`NamedFunction`] +/// itself — the metadata is returned alongside so the collection-level +/// parse can drain it into the [`NamedFunctionMetadataStore`]. +#[derive(Debug)] +pub struct ParsedNamedFunction { + pub named_function: NamedFunction, + pub metadata: NamedFunctionMetadata, +} + impl Parse for v1::NamedFunction { - type Output = NamedFunction; + type Output = ParsedNamedFunction; type Context = (); fn parse(self, _: &Self::Context) -> Result { let message = "ommx.v1.NamedFunction"; - Ok(NamedFunction { + let function = self + .function + .ok_or(RawParseError::MissingField { + message, + field: "function", + })? + .parse_as(&(), message, "function")?; + let named_function = NamedFunction { id: NamedFunctionID(self.id), - function: self - .function - .ok_or(RawParseError::MissingField { - message, - field: "function", - })? - .parse_as(&(), message, "function")?, + function, + }; + let metadata = NamedFunctionMetadata { name: self.name, subscripts: self.subscripts, parameters: self.parameters.into_iter().collect(), description: self.description, + }; + Ok(ParsedNamedFunction { + named_function, + metadata, }) } } impl Parse for Vec { - type Output = BTreeMap; + type Output = ( + BTreeMap, + NamedFunctionMetadataStore, + ); type Context = (); fn parse(self, _: &Self::Context) -> Result { let mut named_functions = BTreeMap::new(); - for named_function in self { - let named_function = named_function.parse(&())?; - let id = named_function.id; - if named_functions.insert(id, named_function).is_some() { + let mut metadata_store = NamedFunctionMetadataStore::default(); + for v in self { + let parsed: ParsedNamedFunction = v.parse(&())?; + let id = parsed.named_function.id; + if named_functions.insert(id, parsed.named_function).is_some() { return Err(RawParseError::InvalidInstance(format!( "Duplicated named function ID is found in definition: {id:?}" )) .into()); } + metadata_store.insert(id, parsed.metadata); } - Ok(named_functions) + Ok((named_functions, metadata_store)) } } -impl From for v1::NamedFunction { - fn from( - NamedFunction { - id, - function, - name, - subscripts, - parameters, - description, - }: NamedFunction, - ) -> Self { - Self { - id: id.into_inner(), - function: Some(function.into()), - name, - subscripts, - parameters: parameters.into_iter().collect(), - description, - } +/// Build a v1 `NamedFunction` from its intrinsic data plus drained metadata. +pub(crate) fn named_function_to_v1( + NamedFunction { id, function }: NamedFunction, + metadata: NamedFunctionMetadata, +) -> v1::NamedFunction { + v1::NamedFunction { + id: id.into_inner(), + function: Some(function.into()), + name: metadata.name, + subscripts: metadata.subscripts, + parameters: metadata.parameters.into_iter().collect(), + description: metadata.description, } } -impl From for v1::EvaluatedNamedFunction { - fn from( - EvaluatedNamedFunction { - id, - evaluated_value, - name, - subscripts, - parameters, - description, - used_decision_variable_ids, - }: EvaluatedNamedFunction, - ) -> Self { - Self { - id: id.into_inner(), - evaluated_value, - name, - subscripts, - parameters: parameters.into_iter().collect(), - description, - used_decision_variable_ids: used_decision_variable_ids - .into_iter() - .map(|id| id.into_inner()) - .collect(), - } - } +/// Parsed v1 `EvaluatedNamedFunction` together with its drained metadata. +#[derive(Debug)] +pub struct ParsedEvaluatedNamedFunction { + pub evaluated_named_function: EvaluatedNamedFunction, + pub metadata: NamedFunctionMetadata, } impl Parse for v1::EvaluatedNamedFunction { - type Output = EvaluatedNamedFunction; + type Output = ParsedEvaluatedNamedFunction; type Context = (); fn parse(self, _: &Self::Context) -> Result { - Ok(EvaluatedNamedFunction { + let evaluated_named_function = EvaluatedNamedFunction { id: NamedFunctionID(self.id), evaluated_value: self.evaluated_value, - name: self.name, - subscripts: self.subscripts, - parameters: self.parameters.into_iter().collect(), - description: self.description, used_decision_variable_ids: self .used_decision_variable_ids .into_iter() .map(VariableID::from) .collect(), + }; + let metadata = NamedFunctionMetadata { + name: self.name, + subscripts: self.subscripts, + parameters: self.parameters.into_iter().collect(), + description: self.description, + }; + Ok(ParsedEvaluatedNamedFunction { + evaluated_named_function, + metadata, }) } } -impl From for v1::SampledNamedFunction { - fn from( - SampledNamedFunction { - id, - evaluated_values, - name, - subscripts, - parameters, - description, - used_decision_variable_ids, - }: SampledNamedFunction, - ) -> Self { - Self { - id: id.into_inner(), - evaluated_values: Some(evaluated_values.into()), - name, - subscripts, - parameters: parameters.into_iter().collect(), - description, - used_decision_variable_ids: used_decision_variable_ids - .into_iter() - .map(|id| id.into_inner()) - .collect(), - } +/// Build a v1 `EvaluatedNamedFunction` from its intrinsic data plus drained metadata. +pub(crate) fn evaluated_named_function_to_v1( + EvaluatedNamedFunction { + id, + evaluated_value, + used_decision_variable_ids, + }: EvaluatedNamedFunction, + metadata: NamedFunctionMetadata, +) -> v1::EvaluatedNamedFunction { + v1::EvaluatedNamedFunction { + id: id.into_inner(), + evaluated_value, + name: metadata.name, + subscripts: metadata.subscripts, + parameters: metadata.parameters.into_iter().collect(), + description: metadata.description, + used_decision_variable_ids: used_decision_variable_ids + .into_iter() + .map(|id| id.into_inner()) + .collect(), } } +/// Parsed v1 `SampledNamedFunction` together with its drained metadata. +#[derive(Debug)] +pub struct ParsedSampledNamedFunction { + pub sampled_named_function: SampledNamedFunction, + pub metadata: NamedFunctionMetadata, +} + impl Parse for v1::SampledNamedFunction { - type Output = SampledNamedFunction; + type Output = ParsedSampledNamedFunction; type Context = (); fn parse(self, _: &Self::Context) -> Result { @@ -160,22 +165,52 @@ impl Parse for v1::SampledNamedFunction { field: "evaluated_values", })? .parse_as(&(), message, "evaluated_values")?; - Ok(SampledNamedFunction { + let sampled_named_function = SampledNamedFunction { id: NamedFunctionID(self.id), evaluated_values, - name: self.name, - subscripts: self.subscripts, - parameters: self.parameters.into_iter().collect(), - description: self.description, used_decision_variable_ids: self .used_decision_variable_ids .into_iter() .map(VariableID::from) .collect(), + }; + let metadata = NamedFunctionMetadata { + name: self.name, + subscripts: self.subscripts, + parameters: self.parameters.into_iter().collect(), + description: self.description, + }; + Ok(ParsedSampledNamedFunction { + sampled_named_function, + metadata, }) } } +/// Build a v1 `SampledNamedFunction` from its intrinsic data plus drained metadata. +pub(crate) fn sampled_named_function_to_v1( + sampled: SampledNamedFunction, + metadata: NamedFunctionMetadata, +) -> v1::SampledNamedFunction { + let SampledNamedFunction { + id, + evaluated_values, + used_decision_variable_ids, + } = sampled; + v1::SampledNamedFunction { + id: id.into_inner(), + evaluated_values: Some(evaluated_values.into()), + name: metadata.name, + subscripts: metadata.subscripts, + parameters: metadata.parameters.into_iter().collect(), + description: metadata.description, + used_decision_variable_ids: used_decision_variable_ids + .into_iter() + .map(|id| id.into_inner()) + .collect(), + } +} + #[cfg(test)] mod tests { use super::*; @@ -193,7 +228,7 @@ mod tests { parameters: Default::default(), description: None, }; - let result: Result = nf.parse(&()); + let result: Result = nf.parse(&()); insta::assert_snapshot!(result.unwrap_err(), @r###" Traceback for OMMX Message parse error: └─ommx.v1.NamedFunction[function] @@ -222,7 +257,7 @@ mod tests { ..Default::default() }, ]; - let result: Result, _> = nfs.parse(&()); + let result: Result<(BTreeMap, _), _> = nfs.parse(&()); insta::assert_snapshot!(result.unwrap_err(), @r###" Traceback for OMMX Message parse error: Duplicated named function ID is found in definition: NamedFunctionID(1) @@ -245,22 +280,24 @@ mod tests { description: Some("A test named function".to_string()), }; - let parsed: EvaluatedNamedFunction = v1_enf.parse(&()).unwrap(); + let parsed: ParsedEvaluatedNamedFunction = v1_enf.parse(&()).unwrap(); + let enf = parsed.evaluated_named_function; + let metadata = parsed.metadata; - assert_eq!(parsed.id(), NamedFunctionID::from(42)); - assert_eq!(parsed.evaluated_value(), 3.14); + assert_eq!(enf.id(), NamedFunctionID::from(42)); + assert_eq!(enf.evaluated_value(), 3.14); assert_eq!( - *parsed.used_decision_variable_ids(), + *enf.used_decision_variable_ids(), btreeset! { VariableID::from(1), VariableID::from(2), VariableID::from(3) } ); - assert_eq!(*parsed.name(), Some("objective_penalty".to_string())); - assert_eq!(*parsed.subscripts(), vec![10, 20]); + assert_eq!(metadata.name, Some("objective_penalty".to_string())); + assert_eq!(metadata.subscripts, vec![10, 20]); assert_eq!( - *parsed.description(), + metadata.description, Some("A test named function".to_string()) ); - assert!(parsed.parameters().contains_key("key1")); - assert_eq!(parsed.parameters()["key1"], "value1"); + assert!(metadata.parameters.contains_key("key1")); + assert_eq!(metadata.parameters["key1"], "value1"); } #[test] @@ -290,20 +327,22 @@ mod tests { used_decision_variable_ids: vec![10, 20], }; - let parsed: SampledNamedFunction = v1_snf.parse(&()).unwrap(); + let parsed: ParsedSampledNamedFunction = v1_snf.parse(&()).unwrap(); + let snf = parsed.sampled_named_function; + let metadata = parsed.metadata; - assert_eq!(*parsed.id(), NamedFunctionID::from(7)); - assert_eq!(parsed.name, Some("cost".to_string())); - assert_eq!(parsed.subscripts, vec![1, 2]); - assert_eq!(parsed.description, Some("A sampled function".to_string())); - assert!(parsed.parameters.contains_key("p")); + assert_eq!(*snf.id(), NamedFunctionID::from(7)); + assert_eq!(metadata.name, Some("cost".to_string())); + assert_eq!(metadata.subscripts, vec![1, 2]); + assert_eq!(metadata.description, Some("A sampled function".to_string())); + assert!(metadata.parameters.contains_key("p")); assert_eq!( - *parsed.used_decision_variable_ids(), + *snf.used_decision_variable_ids(), btreeset! { VariableID::from(10), VariableID::from(20) } ); - // Round-trip: SampledNamedFunction -> v1::SampledNamedFunction - let v1_converted: v1::SampledNamedFunction = parsed.into(); + // Round-trip: SampledNamedFunction + metadata -> v1::SampledNamedFunction + let v1_converted = sampled_named_function_to_v1(snf, metadata); assert_eq!(v1_converted.id, 7); assert_eq!(v1_converted.name, Some("cost".to_string())); assert!(v1_converted.evaluated_values.is_some()); @@ -318,7 +357,7 @@ mod tests { name: Some("f".to_string()), ..Default::default() }; - let result: Result = v1_snf.parse(&()); + let result: Result = v1_snf.parse(&()); insta::assert_snapshot!(result.unwrap_err(), @r###" Traceback for OMMX Message parse error: Field evaluated_values in ommx.v1.SampledNamedFunction is missing. diff --git a/rust/ommx/src/named_function/serialize.rs b/rust/ommx/src/named_function/serialize.rs deleted file mode 100644 index 093d2f5b1..000000000 --- a/rust/ommx/src/named_function/serialize.rs +++ /dev/null @@ -1,118 +0,0 @@ -use super::*; -use crate::{v1, Message, Parse}; -use anyhow::Result; - -impl NamedFunction { - pub fn to_bytes(&self) -> Vec { - let v1_named_function = v1::NamedFunction::from(self.clone()); - v1_named_function.encode_to_vec() - } - - pub fn from_bytes(bytes: &[u8]) -> Result { - let inner = v1::NamedFunction::decode(bytes)?; - Ok(Parse::parse(inner, &())?) - } -} - -impl EvaluatedNamedFunction { - pub fn to_bytes(&self) -> Vec { - let v1_evaluated_named_function = v1::EvaluatedNamedFunction::from(self.clone()); - v1_evaluated_named_function.encode_to_vec() - } - - pub fn from_bytes(bytes: &[u8]) -> Result { - let inner = v1::EvaluatedNamedFunction::decode(bytes)?; - Ok(Parse::parse(inner, &())?) - } -} - -impl SampledNamedFunction { - pub fn to_bytes(&self) -> Vec { - let v1_sampled_named_function = v1::SampledNamedFunction::from(self.clone()); - v1_sampled_named_function.encode_to_vec() - } - - pub fn from_bytes(bytes: &[u8]) -> Result { - let inner = v1::SampledNamedFunction::decode(bytes)?; - Ok(Parse::parse(inner, &())?) - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::{Coefficient, Function}; - - #[test] - fn test_named_function_bytes_roundtrip() { - let nf = NamedFunction { - id: NamedFunctionID::from(5), - function: Function::Constant(Coefficient::try_from(3.14).unwrap()), - name: Some("test_func".to_string()), - subscripts: vec![1, 2, 3], - parameters: { - let mut params = fnv::FnvHashMap::default(); - params.insert("key".to_string(), "value".to_string()); - params - }, - description: Some("roundtrip test".to_string()), - }; - - let bytes = nf.to_bytes(); - let restored = NamedFunction::from_bytes(&bytes).unwrap(); - - assert_eq!(nf, restored); - } - - #[test] - fn test_evaluated_named_function_bytes_roundtrip() { - let enf = EvaluatedNamedFunction { - id: NamedFunctionID::from(10), - evaluated_value: 99.5, - name: Some("eval_func".to_string()), - subscripts: vec![4, 5], - parameters: Default::default(), - description: Some("evaluated roundtrip".to_string()), - used_decision_variable_ids: [crate::VariableID::from(1), crate::VariableID::from(2)] - .into_iter() - .collect(), - }; - - let bytes = enf.to_bytes(); - let restored = EvaluatedNamedFunction::from_bytes(&bytes).unwrap(); - - assert_eq!(enf, restored); - } - - #[test] - fn test_sampled_named_function_bytes_roundtrip() { - let mut evaluated_values = crate::Sampled::default(); - evaluated_values - .append([crate::SampleID::from(0)], 1.0) - .unwrap(); - evaluated_values - .append([crate::SampleID::from(1)], 2.0) - .unwrap(); - - let snf = SampledNamedFunction { - id: NamedFunctionID::from(15), - evaluated_values, - name: Some("sampled_func".to_string()), - subscripts: vec![7, 8, 9], - parameters: { - let mut params = fnv::FnvHashMap::default(); - params.insert("sample_key".to_string(), "sample_value".to_string()); - params - }, - description: Some("sampled roundtrip".to_string()), - used_decision_variable_ids: [crate::VariableID::from(3), crate::VariableID::from(4)] - .into_iter() - .collect(), - }; - - let bytes = snf.to_bytes(); - let restored = SampledNamedFunction::from_bytes(&bytes).unwrap(); - - assert_eq!(snf, restored); - } -} diff --git a/rust/ommx/src/sample_set.rs b/rust/ommx/src/sample_set.rs index 86517deeb..16edf536b 100644 --- a/rust/ommx/src/sample_set.rs +++ b/rust/ommx/src/sample_set.rs @@ -118,6 +118,9 @@ pub struct SampleSet { sos1_constraints: SampledCollection, #[getset(get = "pub")] named_functions: BTreeMap, + /// Per-named-function auxiliary metadata (sibling of [`Self::named_functions`]). + #[getset(get = "pub")] + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, #[getset(get = "pub")] sense: Sense, #[getset(get = "pub")] @@ -274,6 +277,7 @@ impl SampleSet { .evaluated_named_functions(evaluated_named_functions) .decision_variables(decision_variables) .variable_metadata(self.variable_metadata.clone()) + .named_function_metadata(self.named_function_metadata.clone()) .sense(sense) .build_unchecked() .expect("SampleSet invariants guarantee Solution invariants") @@ -358,6 +362,7 @@ pub struct SampleSetBuilder { one_hot_constraints: SampledCollection, sos1_constraints: SampledCollection, named_functions: BTreeMap, + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, sense: Option, } @@ -475,6 +480,15 @@ impl SampleSetBuilder { self } + /// Sets the per-named-function metadata store. + pub fn named_function_metadata( + mut self, + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, + ) -> Self { + self.named_function_metadata = named_function_metadata; + self + } + /// Sets the optimization sense. pub fn sense(mut self, sense: Sense) -> Self { self.sense = Some(sense); @@ -595,6 +609,7 @@ impl SampleSetBuilder { one_hot_constraints: self.one_hot_constraints, sos1_constraints: self.sos1_constraints, named_functions: self.named_functions, + named_function_metadata: self.named_function_metadata.clone(), sense, feasible, feasible_relaxed, @@ -655,6 +670,7 @@ impl SampleSetBuilder { one_hot_constraints: self.one_hot_constraints, sos1_constraints: self.sos1_constraints, named_functions: self.named_functions, + named_function_metadata: self.named_function_metadata.clone(), sense, feasible, feasible_relaxed, diff --git a/rust/ommx/src/sample_set/extract.rs b/rust/ommx/src/sample_set/extract.rs index 8db12c18f..a5eb23fdf 100644 --- a/rust/ommx/src/sample_set/extract.rs +++ b/rust/ommx/src/sample_set/extract.rs @@ -20,8 +20,8 @@ impl SampleSet { /// Returns a set of all unique named function names that have at least one named function. pub fn named_function_names(&self) -> std::collections::BTreeSet { self.named_functions - .values() - .filter_map(|nf| nf.name.clone()) + .keys() + .filter_map(|id| self.named_function_metadata.name(*id).map(str::to_owned)) .collect() } @@ -175,13 +175,13 @@ impl SampleSet { ) -> Result, f64>>, SampleSetError> { let mut result: BTreeMap, f64>> = BTreeMap::new(); - for nf in self.named_functions.values() { - let name = match &nf.name { - Some(n) => n.clone(), + for (id, nf) in &self.named_functions { + let name = match self.named_function_metadata.name(*id) { + Some(n) => n.to_string(), None => continue, // Skip functions without names }; - let subscripts = nf.subscripts.clone(); + let subscripts = self.named_function_metadata.subscripts(*id).to_vec(); let value = *nf .evaluated_values() .get(sample_id) @@ -215,10 +215,11 @@ impl SampleSet { sample_id: SampleID, ) -> Result, f64>, SampleSetError> { // Collect all named functions with the given name - let named_functions_with_name: Vec<&SampledNamedFunction> = self + let named_functions_with_name: Vec<(NamedFunctionID, &SampledNamedFunction)> = self .named_functions - .values() - .filter(|nf| nf.name.as_deref() == Some(name)) + .iter() + .filter(|(id, _)| self.named_function_metadata.name(**id) == Some(name)) + .map(|(id, nf)| (*id, nf)) .collect(); if named_functions_with_name.is_empty() { return Err(SampleSetError::UnknownNamedFunctionName { @@ -226,8 +227,8 @@ impl SampleSet { }); } let mut result = BTreeMap::new(); - for nf in &named_functions_with_name { - let subscripts = nf.subscripts().clone(); + for (id, nf) in &named_functions_with_name { + let subscripts = self.named_function_metadata.subscripts(*id).to_vec(); let value = *nf .evaluated_values() .get(sample_id) @@ -878,6 +879,8 @@ mod tests { } /// Helper to create a SampledNamedFunction via the Parse trait from v1 types. + /// Returns the parsed function plus its drained metadata snapshot, which the + /// caller is responsible for inserting into a host store. fn make_sampled_named_function( id: u64, entries: Vec<(Vec, f64)>, @@ -885,7 +888,7 @@ mod tests { subscripts: Vec, parameters: std::collections::HashMap, description: Option<&str>, - ) -> crate::SampledNamedFunction { + ) -> (crate::SampledNamedFunction, crate::NamedFunctionMetadata) { use crate::parse::Parse; let v1_entries = entries .into_iter() @@ -902,16 +905,20 @@ mod tests { description: description.map(|s| s.to_string()), used_decision_variable_ids: vec![], }; - v1_snf.parse(&()).unwrap() + let parsed: crate::named_function::parse::ParsedSampledNamedFunction = + v1_snf.parse(&()).unwrap(); + (parsed.sampled_named_function, parsed.metadata) } #[test] fn test_extract_named_functions() { // SampleSet with named functions: extract by name + sample_id let mut named_functions = BTreeMap::new(); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); // Named function "cost" with subscript [0] - let snf1 = make_sampled_named_function( + let (snf1, meta1) = make_sampled_named_function( 1, vec![(vec![0], 10.0), (vec![1], 11.0)], Some("cost"), @@ -919,10 +926,12 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(1), snf1); + let id1 = crate::NamedFunctionID::from(1); + named_functions.insert(id1, snf1); + named_function_metadata.insert(id1, meta1); // Named function "cost" with subscript [1] - let snf2 = make_sampled_named_function( + let (snf2, meta2) = make_sampled_named_function( 2, vec![(vec![0], 20.0), (vec![1], 21.0)], Some("cost"), @@ -930,7 +939,9 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(2), snf2); + let id2 = crate::NamedFunctionID::from(2); + named_functions.insert(id2, snf2); + named_function_metadata.insert(id2, meta2); let mut objectives = crate::Sampled::default(); objectives.append([SampleID::from(0)], 0.0).unwrap(); @@ -941,6 +952,7 @@ mod tests { .objectives(objectives) .constraints(BTreeMap::new()) .named_functions(named_functions) + .named_function_metadata(named_function_metadata) .sense(Sense::Minimize) .build() .unwrap(); @@ -985,8 +997,10 @@ mod tests { fn test_extract_named_functions_with_parameters() { // Parameters are now allowed (ignored) - only subscripts are used as keys let mut named_functions = BTreeMap::new(); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); - let snf = make_sampled_named_function( + let (snf, meta) = make_sampled_named_function( 1, vec![(vec![0], 5.0)], Some("f"), @@ -996,7 +1010,9 @@ mod tests { .collect(), None, ); - named_functions.insert(crate::NamedFunctionID::from(1), snf); + let id = crate::NamedFunctionID::from(1); + named_functions.insert(id, snf); + named_function_metadata.insert(id, meta); let mut objectives = crate::Sampled::default(); objectives.append([SampleID::from(0)], 0.0).unwrap(); @@ -1006,6 +1022,7 @@ mod tests { .objectives(objectives) .constraints(BTreeMap::new()) .named_functions(named_functions) + .named_function_metadata(named_function_metadata) .sense(Sense::Minimize) .build() .unwrap(); @@ -1021,9 +1038,11 @@ mod tests { #[test] fn test_extract_all_named_functions() { let mut named_functions = BTreeMap::new(); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); // "cost" [0] - let snf1 = make_sampled_named_function( + let (snf1, meta1) = make_sampled_named_function( 1, vec![(vec![0], 10.0)], Some("cost"), @@ -1031,10 +1050,12 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(1), snf1); + let id1 = crate::NamedFunctionID::from(1); + named_functions.insert(id1, snf1); + named_function_metadata.insert(id1, meta1); // "penalty" [0] - let snf2 = make_sampled_named_function( + let (snf2, meta2) = make_sampled_named_function( 2, vec![(vec![0], 5.0)], Some("penalty"), @@ -1042,10 +1063,12 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(2), snf2); + let id2 = crate::NamedFunctionID::from(2); + named_functions.insert(id2, snf2); + named_function_metadata.insert(id2, meta2); // Unnamed (should be skipped) - let snf3 = make_sampled_named_function( + let (snf3, meta3) = make_sampled_named_function( 3, vec![(vec![0], 99.0)], None, @@ -1053,7 +1076,9 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(3), snf3); + let id3 = crate::NamedFunctionID::from(3); + named_functions.insert(id3, snf3); + named_function_metadata.insert(id3, meta3); let mut objectives = crate::Sampled::default(); objectives.append([SampleID::from(0)], 0.0).unwrap(); @@ -1063,6 +1088,7 @@ mod tests { .objectives(objectives) .constraints(BTreeMap::new()) .named_functions(named_functions) + .named_function_metadata(named_function_metadata) .sense(Sense::Minimize) .build() .unwrap(); @@ -1078,8 +1104,10 @@ mod tests { #[test] fn test_named_function_names() { let mut named_functions = BTreeMap::new(); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); - let snf1 = make_sampled_named_function( + let (snf1, meta1) = make_sampled_named_function( 1, vec![(vec![0], 1.0)], Some("alpha"), @@ -1087,9 +1115,11 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(1), snf1); + let id1 = crate::NamedFunctionID::from(1); + named_functions.insert(id1, snf1); + named_function_metadata.insert(id1, meta1); - let snf2 = make_sampled_named_function( + let (snf2, meta2) = make_sampled_named_function( 2, vec![(vec![0], 2.0)], Some("beta"), @@ -1097,7 +1127,9 @@ mod tests { Default::default(), None, ); - named_functions.insert(crate::NamedFunctionID::from(2), snf2); + let id2 = crate::NamedFunctionID::from(2); + named_functions.insert(id2, snf2); + named_function_metadata.insert(id2, meta2); let mut objectives = crate::Sampled::default(); objectives.append([SampleID::from(0)], 0.0).unwrap(); @@ -1107,6 +1139,7 @@ mod tests { .objectives(objectives) .constraints(BTreeMap::new()) .named_functions(named_functions) + .named_function_metadata(named_function_metadata) .sense(Sense::Minimize) .build() .unwrap(); diff --git a/rust/ommx/src/sample_set/parse.rs b/rust/ommx/src/sample_set/parse.rs index 94f8c0260..09cb1245c 100644 --- a/rust/ommx/src/sample_set/parse.rs +++ b/rust/ommx/src/sample_set/parse.rs @@ -52,12 +52,16 @@ impl Parse for crate::v1::SampleSet { constraints.insert(id, parsed_constraint); } - // Parse named functions into BTreeMap + // Parse named functions into BTreeMap, draining metadata into the SoA store let mut named_functions = std::collections::BTreeMap::new(); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); for v1_named_function in self.named_functions { - let parsed_named_function: crate::SampledNamedFunction = + let parsed: crate::named_function::parse::ParsedSampledNamedFunction = v1_named_function.parse_as(&(), message, "named_functions")?; - named_functions.insert(*parsed_named_function.id(), parsed_named_function); + let id = *parsed.sampled_named_function.id(); + named_functions.insert(id, parsed.sampled_named_function); + named_function_metadata.insert(id, parsed.metadata); } let sense = self.sense.try_into().map_err(|_| { @@ -79,6 +83,7 @@ impl Parse for crate::v1::SampleSet { constraint_metadata, )) .named_functions(named_functions) + .named_function_metadata(named_function_metadata) .sense(sense) .build() .map_err(crate::RawParseError::SampleSetError)?; @@ -164,10 +169,14 @@ impl From for crate::v1::SampleSet { v1_sc }) .collect(); + let named_function_metadata_store = sample_set.named_function_metadata().clone(); let named_functions: Vec = sample_set .named_functions() - .values() - .map(|nf| nf.clone().into()) + .iter() + .map(|(id, nf)| { + let metadata = named_function_metadata_store.collect_for(*id); + crate::named_function::parse::sampled_named_function_to_v1(nf.clone(), metadata) + }) .collect(); let sense = (*sample_set.sense()).into(); @@ -375,13 +384,14 @@ mod tests { fn test_sample_set_roundtrip_preserves_metadata() { use crate::constraint::SampledData; use crate::{ - ATol, ConstraintID, DecisionVariable, Equality, SampleID, SampledDecisionVariable, - Sense, VariableID, + ATol, ConstraintID, DecisionVariable, Equality, NamedFunctionID, SampleID, + SampledDecisionVariable, Sense, VariableID, }; use std::collections::BTreeMap; let var_id = VariableID::from(1); let cid = ConstraintID::from(10); + let nf_id = NamedFunctionID::from(0); let sample_id = SampleID::from(0); let dv = DecisionVariable::binary(var_id); @@ -421,6 +431,35 @@ mod tests { constraint_metadata, ); + // Add a sampled named function with non-empty metadata so the + // round-trip exercises the named_function_metadata SoA store too. + // `SampledNamedFunction` has module-private fields; construct via + // the v1 parse helper (same path Instance::evaluate_samples uses). + let sampled_nf = { + use crate::parse::Parse as _; + let v1_snf = crate::v1::SampledNamedFunction { + id: nf_id.into_inner(), + evaluated_values: Some(crate::v1::SampledValues { + entries: vec![crate::v1::sampled_values::SampledValuesEntry { + ids: vec![sample_id.into_inner()], + value: 1.0, + }], + }), + used_decision_variable_ids: vec![var_id.into_inner()], + ..Default::default() + }; + let parsed: crate::named_function::parse::ParsedSampledNamedFunction = + v1_snf.parse(&()).unwrap(); + parsed.sampled_named_function + }; + let mut named_functions = BTreeMap::new(); + named_functions.insert(nf_id, sampled_nf); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); + named_function_metadata.set_name(nf_id, "offset_x"); + named_function_metadata.set_subscripts(nf_id, vec![0]); + named_function_metadata.set_description(nf_id, "x plus a constant"); + let mut objectives = crate::Sampled::default(); objectives.append([sample_id], 1.0).unwrap(); @@ -429,6 +468,8 @@ mod tests { .variable_metadata(variable_metadata) .objectives(objectives) .constraints_collection(constraints) + .named_functions(named_functions) + .named_function_metadata(named_function_metadata) .sense(Sense::Minimize) .build() .unwrap(); @@ -441,5 +482,9 @@ mod tests { let constraint_meta = recovered.constraints().metadata(); assert_eq!(constraint_meta.name(cid), Some("balance")); assert_eq!(constraint_meta.description(cid), Some("demand-balance row")); + let nf_meta = recovered.named_function_metadata(); + assert_eq!(nf_meta.name(nf_id), Some("offset_x")); + assert_eq!(nf_meta.subscripts(nf_id), &[0]); + assert_eq!(nf_meta.description(nf_id), Some("x plus a constant")); } } diff --git a/rust/ommx/src/solution.rs b/rust/ommx/src/solution.rs index 7b5b240ca..25625b358 100644 --- a/rust/ommx/src/solution.rs +++ b/rust/ommx/src/solution.rs @@ -124,6 +124,9 @@ pub struct Solution { /// Per-variable auxiliary metadata (sibling of [`Self::decision_variables`]). #[getset(get = "pub")] variable_metadata: VariableMetadataStore, + /// Per-named-function auxiliary metadata (sibling of [`Self::evaluated_named_functions`]). + #[getset(get = "pub")] + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, /// Optimality status - not guaranteed by Solution itself pub optimality: crate::v1::Optimality, /// Relaxation status - not guaranteed by Solution itself @@ -446,8 +449,8 @@ impl Solution { /// Named functions without names are not included. pub fn named_function_names(&self) -> BTreeSet { self.evaluated_named_functions - .values() - .filter_map(|nf| nf.name().clone()) + .keys() + .filter_map(|id| self.named_function_metadata.name(*id).map(str::to_owned)) .collect() } @@ -470,8 +473,9 @@ impl Solution { // Collect all named functions with the given name let functions_with_name: Vec<&EvaluatedNamedFunction> = self .evaluated_named_functions - .values() - .filter(|nf| nf.name().as_deref() == Some(name)) + .iter() + .filter(|(id, _)| self.named_function_metadata.name(**id) == Some(name)) + .map(|(_, nf)| nf) .collect(); if functions_with_name.is_empty() { return Err(SolutionError::UnknownNamedFunctionName { @@ -481,7 +485,7 @@ impl Solution { let mut result = BTreeMap::new(); for nf in &functions_with_name { - let key = nf.subscripts().clone(); + let key = self.named_function_metadata.subscripts(nf.id()).to_vec(); if result.contains_key(&key) { return Err(SolutionError::DuplicateSubscript { subscripts: key }); } @@ -505,13 +509,13 @@ impl Solution { ) -> Result, f64>>, SolutionError> { let mut result: BTreeMap, f64>> = BTreeMap::new(); - for nf in self.evaluated_named_functions.values() { - let name = match nf.name() { - Some(n) => n.clone(), + for (id, nf) in &self.evaluated_named_functions { + let name = match self.named_function_metadata.name(*id) { + Some(n) => n.to_string(), None => continue, // Skip named functions without names }; - let subscripts = nf.subscripts().clone(); + let subscripts = self.named_function_metadata.subscripts(*id).to_vec(); let value = nf.evaluated_value(); let funcs_map = result.entry(name).or_default(); @@ -555,6 +559,7 @@ pub struct SolutionBuilder { evaluated_named_functions: BTreeMap, decision_variables: Option>, variable_metadata: VariableMetadataStore, + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, sense: Option, optimality: crate::v1::Optimality, relaxation: crate::v1::Relaxation, @@ -661,6 +666,15 @@ impl SolutionBuilder { self } + /// Sets the per-named-function metadata store. + pub fn named_function_metadata( + mut self, + named_function_metadata: crate::named_function::NamedFunctionMetadataStore, + ) -> Self { + self.named_function_metadata = named_function_metadata; + self + } + /// Sets the optimization sense. pub fn sense(mut self, sense: Sense) -> Self { self.sense = Some(sense); @@ -785,6 +799,7 @@ impl SolutionBuilder { evaluated_named_functions: self.evaluated_named_functions, decision_variables, variable_metadata: self.variable_metadata.clone(), + named_function_metadata: self.named_function_metadata.clone(), optimality: self.optimality, relaxation: self.relaxation, sense: Some(sense), @@ -834,6 +849,7 @@ impl SolutionBuilder { evaluated_named_functions: self.evaluated_named_functions, decision_variables, variable_metadata: self.variable_metadata.clone(), + named_function_metadata: self.named_function_metadata.clone(), optimality: self.optimality, relaxation: self.relaxation, sense: Some(sense), diff --git a/rust/ommx/src/solution/parse.rs b/rust/ommx/src/solution/parse.rs index f7a6d6e0b..af25d9580 100644 --- a/rust/ommx/src/solution/parse.rs +++ b/rust/ommx/src/solution/parse.rs @@ -53,9 +53,14 @@ impl Parse for crate::v1::Solution { evaluated_constraints.insert(id, parsed_constraint); } let mut evaluated_named_functions = std::collections::BTreeMap::default(); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); for enf in self.evaluated_named_functions { - let parsed_named_function = enf.parse_as(&(), message, "evaluated_named_functions")?; - evaluated_named_functions.insert(parsed_named_function.id(), parsed_named_function); + let parsed: crate::named_function::parse::ParsedEvaluatedNamedFunction = + enf.parse_as(&(), message, "evaluated_named_functions")?; + let id = parsed.evaluated_named_function.id(); + evaluated_named_functions.insert(id, parsed.evaluated_named_function); + named_function_metadata.insert(id, parsed.metadata); } let mut decision_variables = std::collections::BTreeMap::default(); @@ -121,6 +126,7 @@ impl Parse for crate::v1::Solution { evaluated_named_functions, decision_variables, variable_metadata, + named_function_metadata, optimality, relaxation, sense, @@ -188,10 +194,14 @@ impl From for crate::v1::Solution { v1_ec }) .collect(); - let evaluated_named_functions = solution + let named_function_metadata_store = solution.named_function_metadata().clone(); + let evaluated_named_functions: Vec = solution .evaluated_named_functions() - .values() - .map(|enf| enf.clone().into()) + .iter() + .map(|(id, enf)| { + let metadata = named_function_metadata_store.collect_for(*id); + crate::named_function::parse::evaluated_named_function_to_v1(enf.clone(), metadata) + }) .collect(); let variable_metadata_store = solution.variable_metadata().clone(); let decision_variables: Vec = solution @@ -489,13 +499,14 @@ mod tests { fn test_solution_roundtrip_preserves_metadata() { use crate::{ constraint::EvaluatedData, constraint_type::EvaluatedCollection, ATol, ConstraintID, - DecisionVariable, Equality, EvaluatedConstraint, EvaluatedDecisionVariable, Sense, - VariableID, + DecisionVariable, Equality, EvaluatedConstraint, EvaluatedDecisionVariable, + NamedFunctionID, Sense, VariableID, }; use std::collections::BTreeMap; let var_id = VariableID::from(1); let cid = ConstraintID::from(10); + let nf_id = NamedFunctionID::from(0); let dv = DecisionVariable::binary(var_id); let evaluated_dv = EvaluatedDecisionVariable::new(dv, 1.0, ATol::default()).unwrap(); @@ -523,16 +534,42 @@ mod tests { let evaluated_constraints = EvaluatedCollection::with_metadata(evaluated_map, BTreeMap::new(), constraint_metadata); + // Add an evaluated named function with non-empty metadata so the + // round-trip exercises the named_function_metadata SoA store too. + // Construct via the v1 parse helper because + // `used_decision_variable_ids` is module-private on + // `EvaluatedNamedFunction`. + let evaluated_nf = { + use crate::parse::Parse as _; + let v1_enf = crate::v1::EvaluatedNamedFunction { + id: nf_id.into_inner(), + evaluated_value: 1.0, + used_decision_variable_ids: vec![var_id.into_inner()], + ..Default::default() + }; + let parsed: crate::named_function::parse::ParsedEvaluatedNamedFunction = + v1_enf.parse(&()).unwrap(); + parsed.evaluated_named_function + }; + let mut evaluated_named_functions = BTreeMap::new(); + evaluated_named_functions.insert(nf_id, evaluated_nf); + let mut named_function_metadata = + crate::named_function::NamedFunctionMetadataStore::default(); + named_function_metadata.set_name(nf_id, "offset_x"); + named_function_metadata.set_subscripts(nf_id, vec![0]); + named_function_metadata.set_description(nf_id, "x plus a constant"); + // SAFETY: the inputs above satisfy Solution invariants (one DV, - // one evaluated constraint over that DV, value 1.0 satisfies the - // equality, no removed reasons). + // one evaluated constraint over that DV, one named function over + // that DV, value 1.0 satisfies the equality, no removed reasons). let solution = unsafe { Solution::builder() .objective(1.0) .evaluated_constraints_collection(evaluated_constraints) - .evaluated_named_functions(BTreeMap::new()) + .evaluated_named_functions(evaluated_named_functions) .decision_variables(decision_variables) .variable_metadata(variable_metadata) + .named_function_metadata(named_function_metadata) .sense(Sense::Minimize) .build_unchecked() .unwrap() @@ -546,5 +583,9 @@ mod tests { let constraint_meta = recovered.evaluated_constraints().metadata(); assert_eq!(constraint_meta.name(cid), Some("balance")); assert_eq!(constraint_meta.description(cid), Some("demand-balance row")); + let nf_meta = recovered.named_function_metadata(); + assert_eq!(nf_meta.name(nf_id), Some("offset_x")); + assert_eq!(nf_meta.subscripts(nf_id), &[0]); + assert_eq!(nf_meta.description(nf_id), Some("x plus a constant")); } }