Skip to content

fix KeyError: 'weight_mode' when packing v2.1 recipes with under-specified gradients#470

Open
rugeli wants to merge 4 commits into
mainfrom
gradient-validation
Open

fix KeyError: 'weight_mode' when packing v2.1 recipes with under-specified gradients#470
rugeli wants to merge 4 commits into
mainfrom
gradient-validation

Conversation

@rugeli
Copy link
Copy Markdown
Collaborator

@rugeli rugeli commented Jun 1, 2026

Problem

closes #465

Sorry for the delay on this one, after tracing it back through the packing pipeline, I figured the gradient packing error is not a problem with our Pydantic schema. The root cause is a data-pipeline gap. Gradient defaults were only ever filled in one place: the v2.0→v2.1 migration. But recipe_loader._read only runs migration when format_version != current_version. A recipe hand-authored directly as 2.1 skips migration entirely, so its gradients reach Gradient.__init__ still missing keys.

In short, without the changes in this pr, relabeling format_version as 2.0, everything else stays the same, the recipe cellpack/tests/recipes/v2/test_peroxisome_combined_gradient.json would pass.

Solution

in recipe_loader:

  • added gradient normalization that converts list-of-dict form and fill missing top-level keys. Made this conversion happen before validation.
  • removed now-redundant gradient dict to list block

with @mogres

Note: If we want to further refactor, we can:

  1. make use of the validator output. validate_recipe builds a normalized copy that _read currently discards, we currently only use validator for validating which makes sense to me. But if we want to, we can make the validated recipe source of truth. This will be a larger pr so I'd keep this separate.
  2. unify version migration scripts(default filling, structure flatten&nested, etc). more files involved, so better be another separate pr

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Steps to Verify:

  1. run pack -r cellpack/tests/recipes/v2/test_peroxisome_combined_gradient.json -c cellpack/tests/packing-configs/test_gradient_mixing.json

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 1, 2026

Packing analysis report

Analysis for packing results located at cellpack/tests/outputs/test_spheres/spheresSST

Ingredient name Encapsulating radius Average number packed
ext_A 25 236.0

Packing image

Packing image

Distance analysis

Expected minimum distance: 50.00
Actual minimum distance: 50.01

Ingredient key Pairwise distance distribution
ext_A Distance distribution ext_A

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a packing-time KeyError: 'weight_mode' for v2.1 recipes whose gradients are under-specified (i.e., they skip v2.0→v2.1 migration, so gradient defaults never get filled). The core change is to normalize gradients earlier in RecipeLoader._read() so gradients are consistently list-of-dicts and have required top-level keys before validation and before constructing Gradient instances.

Changes:

  • Add RecipeLoader._normalize_gradients() to convert dict→list gradient format and fill missing top-level gradient keys from defaults prior to validation.
  • Remove the older late-stage “gradients dict→list” conversion block and move serializable_recipe_data capture to occur after dict-level normalization.
  • Add a reproduction recipe/config under cellpack/tests/ for manual verification of gradient-mixing packing.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
cellpack/autopack/loaders/recipe_loader.py Adds gradient normalization + reorders normalization/validation/serialization to avoid missing gradient keys in v2.1 recipes.
cellpack/tests/recipes/v2/test_peroxisome_combined_gradient.json Adds a v2.1 recipe fixture demonstrating combined gradients / under-specified gradient keys.
cellpack/tests/packing-configs/test_gradient_mixing.json Adds a packing config used to reproduce the gradient-mixing scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread cellpack/autopack/loaders/recipe_loader.py
Comment thread cellpack/tests/recipes/v2/test_peroxisome_combined_gradient.json
Comment thread cellpack/autopack/loaders/recipe_loader.py
@rugeli rugeli requested review from ascibisz, meganrm and mogres June 1, 2026 20:46
@rugeli rugeli force-pushed the gradient-validation branch from 592a091 to 460ab53 Compare June 4, 2026 01:05
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.

Validation schema improvements

4 participants