Skip to content

Guard raw-backend accessors on degenerate fits and bad transform input #306

Closed

Description

@neuromechanist

From the PR #305 review (epic #278 Phase 1). Three gaps exist identically in AMICATorchNG and (by deliberate parity) AMICAMLXNG; fixing them one-backend-only would violate .rules/backend_parity.md, so they are tracked here for one cross-backend PR:

  1. transform/get_mixing_matrix/get_unmixing_matrix/get_sensor_mixing_matrix return numeric-looking output from a fit the backend itself classified as degenerate (stop_reason in the degenerate set, or a non-finite parameter): no warning, no error. get_rho already carries a finiteness guard on both backends, and state_dict refuses to persist such a model, so the fix pattern exists. The AMICA wrapper's AMICA wrapper marks is_fitted_ even on a degenerate (nan_ll/singular_ll) fit #50 contract (_check_usable) already protects wrapper users; this is about the raw backend surface.

  2. transform(X) does not validate X.ndim/X.shape[0] the way fit does on the same classes, so a wrong-channel-count array fails with a raw matmul/broadcast error instead of the named ValueError fit gives for the identical mistake.

  3. from_state_dict's cls(**config) surfaces malformed config as a bare TypeError (unexpected/missing kwargs) while every neighboring validation step raises a named ValueError identifying the save file as the culprit.

Same-PR cross-backend fix with a shared test per .rules/backend_parity.md.

Activity

  1. neuromechanist commented on Sep 1, 2026

    @neuromechanist
    MemberAuthor

    Scope extension from the PR #311 review: the Phase 3 scoring accessors reproduce the same anti-pattern and postdate this issue's filing, so adding them explicitly: model_loglik/model_probability on BOTH backends (torch core.py ~3029-3106 and the MLX ports) lack the degenerate-fit/finiteness guard and the X.ndim/X.shape[0] validation, same as the item-1/item-2 accessors already listed. Also folding in: model_probability's all-non-finite error message says "-inf log-likelihood" but the isfinite check equally fires on NaN (numerical corruption), misdiagnosing the cause -- split or reword the message when fixing item 1 here. One cross-backend PR for the lot, per .rules/backend_parity.md.

  2. neuromechanist commented on Sep 1, 2026

    @neuromechanist
    MemberAuthor

    Second scope extension, from the epic #278 parity audit: AMICA_NumPy.transform() (numpy_impl/core.py ~2498-2528) has the identical degenerate-fit hole — it never checks self.converged/stop_reason, so a diverged legacy-backend fit silently returns NaN sources. The issue's fix should cover all three backends, not two. (The numpy CLI's main() does check convergence before declaring success; the library path does not.)

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