Skip to content

Use om.Constraints throughout.#86

Open
hmgaudecker wants to merge 5 commits intooutput-formattingfrom
om.Constraints
Open

Use om.Constraints throughout.#86
hmgaudecker wants to merge 5 commits intooutput-formattingfrom
om.Constraints

Conversation

@hmgaudecker
Copy link
Member

@hmgaudecker hmgaudecker commented Mar 14, 2026

We still had the dict-based constraints from estimagic in here and converted to modern constraints only in the very last step. This PR switches to the optimagic constraints.

Main realisation: optimagic just picks whatever start value there is; but skillmodels needs to set the start value for fixed constraints. Hence we introduce a simple wrapper class:

@dataclass(frozen=True)
class FixedConstraintWithValue(om.FixedConstraint):
    """Fixed constraint that carries the target value and parameter location.

    `om.FixedConstraint` fixes parameters at their start values but does not carry a
    target value. This wrapper adds `loc` (the parameter location in the params
    DataFrame) and `value` (the value to set before optimization).
    """

    loc: pd.MultiIndex | tuple | str | None = None
    """Parameter location in the params DataFrame."""
    value: float | None = None
    """Value to enforce on the parameter."""

    def __post_init__(self) -> None:
        """Validate that `loc` and `value` are not None and derive `selector`."""
        if self.loc is None:
            msg = "loc must not be None"
            raise TypeError(msg)
        if self.value is None:
            msg = "value must not be None"
            raise TypeError(msg)
        object.__setattr__(
            self,
            "selector",
            functools.partial(select_by_loc, loc=self.loc),
        )

@codecov
Copy link

codecov bot commented Mar 14, 2026

Codecov Report

❌ Patch coverage is 94.47853% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.10%. Comparing base (67d7b44) to head (a32a177).

Files with missing lines Patch % Lines
src/skillmodels/constraints.py 93.24% 5 Missing ⚠️
tests/test_constraints.py 93.61% 3 Missing ⚠️
src/skillmodels/transition_functions.py 95.23% 1 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##           output-formatting      #86      +/-   ##
=====================================================
- Coverage              97.28%   97.10%   -0.19%     
=====================================================
  Files                     57       57              
  Lines                   4796     4761      -35     
=====================================================
- Hits                    4666     4623      -43     
- Misses                   130      138       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hmgaudecker and others added 4 commits March 15, 2026 14:47
Remove list from loc type union, convert callers to tuple().
Update anchoring test expectations from list to tuple.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@hmgaudecker hmgaudecker marked this pull request as ready for review March 15, 2026 13:50
@hmgaudecker hmgaudecker requested a review from janosg March 15, 2026 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant