Skip to content

fix(config): reject unsupported bifactor JMLE before fitting #643

Description

@seonghobae

Public contract defect

Protected main a7be9eb8ca0b807364173f8471fc300e3cf9350f accepts FitConfig(model="BIFAC2PLM", estimator="jmle") through FitConfig.validate(), but public fit() later raises NotImplementedError because BIFAC2PLM is supported only by the marginal/MMLE estimator. This is the same API-design failure class as #639, but it is a distinct model × estimator compatibility constraint rather than an unimplemented estimator identity.

A production configuration object that reports itself valid must not later reveal that the selected model/estimator combination is an unimplemented stub. The unsupported combination should fail at configuration/compatibility validation with a stable bounded ValueError before response preparation or fitting work.

Sequencing / writer lease

PR #640 currently owns python/fast_mlsirm/config.py while narrowing the estimator vocabulary. Do not create a competing source branch or edit the same validation surface while #640 has an active writer. After #640 stabilizes or integrates, refetch protected main and reuse its final vocabulary/error conventions.

Test-first acceptance

  1. Add a realistic RED at the public configuration/fit boundary proving BIFAC2PLM + jmle is rejected by validation/compatibility preflight, not later by NotImplementedError inside fit().
  2. Preserve BIFAC2PLM + mmle exactly, including existing Rust-owned bifactor likelihood/calibration semantics and scoreability boundaries.
  3. Preserve all currently supported non-bifactor JMLE/MMLE model combinations; do not infer future compatibility from model names alone.
  4. Prefer one explicit model-estimator compatibility table/policy with a finite stable error surface over scattered late if ...: raise NotImplementedError branches.
  5. The compatibility error must be non-reflective with respect to caller data and must run before response materialization/numerical work.
  6. Remove the now-unreachable late NotImplementedError or retain only a defensive assertion/internal invariant that cannot be the normal public path.
  7. Add exhaustive matrix tests over every current VALID_MODELS × VALID_ESTIMATORS supported/unsupported pair so future model additions cannot silently advertise unusable combinations.
  8. Complete public docstrings and exact changed-production statement/branch coverage; add authoritative changelog/doctoring and render/check the managed changelog.
  9. Require one unchanged head with full Python/Rust/PyO3/package/GPU/fuzz/Security/SAST/current-head review evidence before Ready.

Non-goals

Closure

Close only after protected main validates the current model × estimator compatibility matrix up front, BIFAC2PLM + jmle cannot reach an advertised-but-unimplemented fitting path, and supported combinations remain behaviorally unchanged.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions