Skip to content

Commit cde6d90

Browse files
janosgjanosg
committed
Convert all algorithm documentation to the new format.
1 parent 79936a6 commit cde6d90

17 files changed

Lines changed: 9115 additions & 3378 deletions

‎CLEANUP_NOTES.MD‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
Here we collect task to work on after the main refactoring of constraints is done.
2+
3+
4+
## Bug Fixes
5+
6+
7+
8+
## Improvements
9+
10+
- Check whether there are more cases like diagonal covariance matrices or 1x1 covariance matrices that can actually be covered without A reparametrization. A good candidate are probability constraints with 2 or less entries. I haven't checked if this is already exploited.
11+
- Update and extend the documentation (how constraints are implemented)
12+
- Do we really need the flat param names for error messages in ResolutionContext if we have the source and can apply selector to params?
13+
14+
15+
16+
## Clean UP
17+
18+
- Remove all mentions of the constraints refactoring (e.g. in test_constraint_pipeline_invariants)
19+
- The IntArray and FloatArray types in optimagic.parameters.constraints.types should actually be defined in optimagic.typing and used throughout.
20+
- Split up constraints.py into one file per constraint type that also contains the transformations

‎CONSTRAINTS_REFACTOR_REVIEW.md‎

Lines changed: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
1+
# Review guide: constraints refactoring stack
2+
3+
Context document for Claude sessions that review one PR of the constraints
4+
refactoring. Read this first, then check out the branch under review. This file is
5+
deliberately untracked; delete it when the whole stack is merged.
6+
7+
## Status (2026-07-03, evening)
8+
9+
- PRs 1-3 are **merged into `main`** (squash: #686, #687, #688). `main` is the base
10+
of the remaining stack.
11+
- Branches 4-6 have been rebased onto the new `main` (each squash drops the original
12+
commits, so this used `git rebase --onto origin/main <old-branch-tip> <branch>`
13+
per branch). All remaining branches contain their predecessor and are green on
14+
the targeted test loop, mypy, and pre-commit; the full fast suite passed on the
15+
last branch.
16+
- PR 3 review landed major design changes (now in `main`): selector resolution is a
17+
`Constraint._resolve(context)` method on each user constraint class; the
18+
`Resolved*Constraint` dataclasses live in `optimagic.constraints` directly after
19+
their user-facing counterparts (`parameters/constraints/types.py` no longer
20+
exists); resolved class names exactly match their user counterparts
21+
(e.g. `ResolvedFlatCovConstraint`); resolving a `NonlinearConstraint` raises
22+
`NotImplementedError`. Constraint-type-specific behavior is unit tested in
23+
`tests/optimagic/test_constraints.py`; `test_resolution.py` covers only general
24+
end-to-end resolution behavior.
25+
26+
## What this refactoring is
27+
28+
The code that implements constraints via reparametrization (formerly
29+
`process_constraints.py`, `consolidate_constraints.py`, `check_constraints.py`,
30+
`process_selectors.py`, `kernel_transformations.py`, `space_conversion.py` in
31+
`src/optimagic/parameters/`) was rewritten in 5 stacked PRs. Goals: typed internals
32+
(frozen dataclasses instead of the mutable `constr_info` dict and constraint dicts),
33+
less entanglement, provenance-based error messages that cite the user's original
34+
constraints, and extension slots for kernel second derivatives, jax derivatives,
35+
probability-with-fixed-params, and native linear-constraint pass-through (none of
36+
these extensions are implemented; do not add them during review).
37+
38+
The full design and rationale are in the plan file
39+
`~/.claude/plans/i-want-to-refactor-moonlit-nygaard.md`. The math is unchanged and
40+
documented in `docs/source/explanation/implementation_of_constraints.md`.
41+
42+
## The branch stack (review in this order)
43+
44+
Each branch is one PR, sits on top of the previous one, and is green on the full
45+
fast test suite, mypy, and pre-commit.
46+
47+
1. `constraints-refactor-1-characterization-tests` — **MERGED (squash, #686)**; now
48+
the base of the stack.
49+
Test-only safety net: `tests/optimagic/parameters/test_constraint_pipeline_invariants.py`
50+
pins golden internal params for 26 constraint sets, round trips, the feasibility
51+
invariant, derivatives, and first-error types at the `get_converter` seam. During
52+
review the corpus grew (shifted covariance blocks, sdcorr simplified to bounds,
53+
uncorrelated sdcorr) and the fixtures were made typed/frozen (a `Case` dataclass
54+
and an `ExpectedInternal` dataclass instead of dicts).
55+
Also carries two behavior fixes: `check_fixes_and_bounds` crashed with `TypeError`
56+
instead of raising `InvalidConstraintError`; and the uncorrelated-covariance
57+
simplification now works for covariance blocks that do not start at position 0 of
58+
the flat vector (the off-diagonal indices are selected by block-local position, not
59+
by matching global index values). The second fix was moved up from PR 6.
60+
61+
2. `constraints-refactor-2-typed-boundary`
62+
`deprecations.pre_process_constraints` inverted: legacy dict constraints
63+
(incl. `loc/locs/query/queries`, which become selector closures) are converted
64+
INTO `Constraint` objects; `OptimizationProblem.constraints` is
65+
`list[Constraint]`; nonlinear split via `isinstance`. Temporary `_to_dict` seams
66+
keep the old dict pipeline running (removed in PR 5).
67+
New tests were added to `tests/optimagic/test_deprecations.py` (not a separate
68+
file: they cover `deprecations.pre_process_constraints`, so they can be deleted
69+
together with the rest of the deprecation-era tests once dict constraints are
70+
removed in 0.6.0).
71+
72+
3. `constraints-refactor-3-typed-resolution` — **MERGED (squash, #688)**.
73+
Selector resolution as typed `Constraint._resolve(context)` methods; the
74+
`Resolved*Constraint` dataclasses and `ConstraintSource` provenance live in
75+
`optimagic.constraints`; `resolution.py` holds the loop and `ResolutionContext`.
76+
`process_selectors.py` deleted. A temporary `to_legacy_dicts` seam feeds the
77+
still-dict-based consolidation (removed in PR 5).
78+
79+
4. `constraints-refactor-4-numpy-linear-consolidation`
80+
Shape-preserving pandas -> numpy rewrite of the linear consolidation, verified
81+
by a 200-scenario differential test against a verbatim copy of the pandas code
82+
(`tests/optimagic/parameters/test_linear_consolidation_differential.py`,
83+
deleted again in PR 5). Preserves first-occurrence dedup order and lb/ub swaps
84+
under negative rescaling; linear constraints no longer get their index rewritten
85+
during equality plugging (the rewritten index was unused before but would have
86+
corrupted the numpy weight scatter).
87+
88+
5. `constraints-refactor-5-typed-core`
89+
The big one: `validation.py`, `consolidation.py`, `consolidate_linear.py`,
90+
`transforms.py`, `kernels.py`; `SpaceConversionSpec` replaces `constr_info`;
91+
`SpaceConverter` gets real methods; `get_space_converter` orchestrates
92+
validate -> normalize -> consolidate. Old modules, all seams, and
93+
`Constraint._to_dict` are deleted. Test retargeting changed only constraint
94+
construction lines and imports — no expected value changed.
95+
96+
6. `constraints-refactor-6-review-fixes`
97+
Collects genuine behavior bugs found during review, one commit + regression test
98+
each. The uncorrelated-covariance-on-shifted-blocks fix now lands in PR 1 (`main`)
99+
and is carried through the typed core in PR 5, so this PR now only adds the
100+
dedicated consolidation-stage regression test (`test_consolidation.py`).
101+
102+
## Ground rules for review sessions
103+
104+
- **Golden values are the contract.** Never edit expected numbers in
105+
`test_constraint_pipeline_invariants.py` or `test_space_conversion.py`. If a
106+
change would require that, it changes the reparametrization and needs explicit
107+
discussion, not a test update.
108+
- **Deliberate behavior changes — do not revert them:**
109+
- fixes/bounds colliding with cov/sdcorr/probability raise
110+
`InvalidConstraintError` (was `TypeError`, PR 1),
111+
- conflicting fixes on equality-constrained params raise
112+
`InvalidConstraintError` (was `AssertionError`, PR 5),
113+
- conflicting linear fixed values raise `InvalidConstraintError`
114+
(was `ValueError`, PR 5),
115+
- misaligned linear weights raise `InvalidConstraintError` at resolution time
116+
(was `ValueError` later, PR 3),
117+
- resolving a `NonlinearConstraint` raises `NotImplementedError` (was
118+
`InvalidConstraintError`, PR 3): nonlinear constraints are passed directly to
119+
optimizers and are split off before resolution, so this is an internal error,
120+
not invalid user input,
121+
- post-consolidation error messages cite the originating user constraints.
122+
- **Where changes go:**
123+
- Review edits to PR N (naming, docstrings, structure, behavior-preserving
124+
refactors) -> new commit on branch N.
125+
- Genuine behavior bugs (pre-existing or introduced) -> branch
126+
`constraints-refactor-6-review-fixes`, one commit per fix with a regression
127+
test. Not on the PR branches, so those stay behavior-preserving.
128+
129+
## Propagation rule (important)
130+
131+
Every change committed to a branch during review MUST be merged into all later
132+
branches of the stack by rebasing the rest of the stack. It is enough to do this
133+
ONCE at the end of the session (not after every commit):
134+
135+
```bash
136+
# after all review changes on constraints-refactor-<N>-... are committed:
137+
git switch constraints-refactor-<N+1>-...
138+
git rebase constraints-refactor-<N>-...
139+
# repeat for each later branch in order, up to and including
140+
# constraints-refactor-6-review-fixes
141+
```
142+
143+
After rebasing, rerun the verification (below) on the LAST branch of the stack, not
144+
just the branch under review. Never leave the session with the stack in a state
145+
where a later branch does not contain an earlier branch.
146+
147+
## Verification commands
148+
149+
```bash
150+
# fast targeted loop (seconds)
151+
pixi run pytest tests/optimagic/parameters tests/optimagic/test_constraints.py \
152+
tests/optimagic/test_deprecations.py \
153+
tests/optimagic/optimization/test_with_constraints.py \
154+
tests/optimagic/optimization/test_with_advanced_constraints.py \
155+
tests/estimagic -m "not slow"
156+
157+
# full fast suite (~4-5 min), strict typing, hooks
158+
pixi run tests-fast
159+
pixi run mypy
160+
pre-commit run --all-files
161+
```
162+
163+
All new modules under `src/optimagic/parameters/constraints/` and the rewritten
164+
`space_conversion.py` are strictly type checked (not in the mypy override exempt
165+
list in `pyproject.toml`) — keep it that way.
166+
167+
## Known context that saves time
168+
169+
- Legacy dict constraints are deprecated (FutureWarning, removal in 0.6.0). The
170+
adapter in `deprecations.py` (incl. `FixedValueConstraint` and the loc/query
171+
selector closures) exists only to support them and dies with them.
172+
- `InternalParams` and `SpaceConverter` still live in
173+
`parameters/space_conversion.py` on purpose: import paths for estimagic, the
174+
slice plots and `constraint_tools` are unchanged.
175+
- Consolidation order in `consolidation.py::consolidate_constraints` is load
176+
bearing (equality merging -> fix propagation -> cov/sdcorr simplification ->
177+
bound tightening -> replacements -> dedup -> linear bundling -> checks -> spec
178+
assembly). Do not reorder while reviewing for style.
179+
- The tests directory has no `__init__.py` files: test module basenames must be
180+
unique across the whole tests tree.

0 commit comments

Comments
 (0)