Skip to content

Fix bulk-id generation in analyze_traps and enforce MP-edge diagnostics#396

Open
charlesmartin14 wants to merge 3 commits intomasterfrom
codex/fix-weightwatcher-bulk-id-api
Open

Fix bulk-id generation in analyze_traps and enforce MP-edge diagnostics#396
charlesmartin14 wants to merge 3 commits intomasterfrom
codex/fix-weightwatcher-bulk-id-api

Conversation

@charlesmartin14
Copy link
Copy Markdown
Member

Motivation

  • analyze_traps(..., return_bulk_ids=True, bulk_only=True) was returning zero bulk rows because the MP upper edge (mp_bulk_max) was not being read/wired into the bulk-row builder, causing the MP membership test to fail and silently drop all bulk modes.
  • Bulk IDs must be public, stable (1-based), map to internal 0-based SVD indices, and never be mixed with trap IDs, while allowing controlled fallback behavior when MP edges are missing.

Description

  • Add a robust helper _extract_mp_bulk_edges(...) to read MP edge candidates from layer_state, per-row details, or bulk_stats and return (mp_bulk_min, mp_bulk_max).
  • Wire MP edge values into layer_state and bulk_stats before building bulk rows and persist evals and singular_values there, so bulk eligibility sees the real edges and spectrum (layer_state['mp_bulk_min'], layer_state['mp_bulk_max'], layer_state['evals'], layer_state['singular_values']).
  • Compute bulk eligibility by excluding trap SVD indices, requiring finite eigenvalues, and applying mp_bulk_min <= eval <= mp_bulk_max when edges are present; assign 1-based bulk_id -> internal svd_mode_index mappings in layer_state['bulk_id_to_svd_index'] and keep trap/bulk fields separate in returned rows.
  • Add an explicit diagnostic ValueError when return_bulk_ids=True on a nondegenerate layer and no eligible bulk modes are found, and expose allow_bulk_without_mp_edges (default False) to permit controlled fallback to non-trap finite modes.
  • Enrich bulk row metadata with mp_bulk_min, mp_bulk_max, mp_bulk_* fields and bulk_quantile, and forward the new allow_bulk_without_mp_edges option through WeightWatcher.analyze_traps.

Testing

  • Added a regression test tests/test_bulk_mode_ids.py::test_bulk_only_has_mp_edges_and_nonempty that asserts bulk_only=True returns non-empty bulk rows and that out_state['layers'][..] contains finite mp_bulk_min/mp_bulk_max and non-empty bulk_svd_indices.
  • Ran pytest -q tests/test_bulk_mode_ids.py in this environment; the run was skipped because torch is unavailable (output: 1 skipped).
  • Local logic coverage validated via updated code paths exercising _build_trap_bulk_rows, MP edge extraction, and bulk id mapping (no other automated test failures observed in the scoped run).

Codex Task

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant