Skip to content

fix(diloco test): set enable_checkpointing explicitly when loading state - #5132

Open
gulsumgudukbay wants to merge 1 commit into
AI-Hypercomputer:mainfrom
ROCm:fix-diloco-decoupled-checkpointing
Open

fix(diloco test): set enable_checkpointing explicitly when loading state#5132
gulsumgudukbay wants to merge 1 commit into
AI-Hypercomputer:mainfrom
ROCm:fix-diloco-decoupled-checkpointing

Conversation

@gulsumgudukbay

@gulsumgudukbay gulsumgudukbay commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Description

test_diloco_checkpoint_saving_and_normal_resume builds a third config to verify normal (non-DiLoCo) full-state restoration. It sets load_full_state_path but relies on the base config file for enable_checkpointing, and config validation
rejects that combination: "You must set enable_checkpointing=True to load a checkpoint."

base.yml defaults the flag to true, so the test passes today. But get_test_config_path() resolves to decoupled_base_test.yml when DECOUPLE_GCLOUD=TRUE, and that file sets enable_checkpointing: false, so the
test fails validation there. The first config in the same test already passes the flag explicitly; this makes the third one do the same instead of depending on which base file happens to be selected.

Tests

Reproduce the failure on main and the fix on this branch with:

DECOUPLE_GCLOUD=TRUE python -m pytest \
  tests/integration/diloco_test.py -k test_diloco_checkpoint_saving_and_normal_resume

Verified on 4x AMD Instinct MI355X (ROCm 10, jax 0.11.1) with DECOUPLE_GCLOUD=TRUE: the test fails on main
with the validation error above and passes on this branch.

Checklist

  • I have performed a self-review of my code. For an optional AI review, add the gemini-review label.
  • I have necessary comments in my code, particularly in hard-to-understand areas.
  • I have run end-to-end tests tests and provided workload links above if applicable.
  • I have made or will make corresponding changes to the doc if needed.

`load_full_state_path` requires `enable_checkpointing=true`, and the third
config in this test relied on the base file to supply it. `base.yml` defaults it
to true but `decoupled_base_test.yml` sets it to false, so the test fails config
validation whenever `get_test_config_path()` resolves to the decoupled base. The
first config in the same test already passes the flag explicitly; do the same
here rather than depend on the base default.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the integration test test_diloco_checkpoint_saving_and_normal_resume in tests/integration/diloco_test.py by adding the enable_checkpointing=true configuration parameter. There are no review comments, and I have no feedback to provide.

@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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.

3 participants