Draft
Conversation
github-actions
bot
added
module: engine
goanpeca
force-pushed
the
fix/max-iters
branch
from
f62141f to
6f9c0f4
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements a fix for max_iters handling in the Engine state dictionary and adds comprehensive test coverage. The changes extend the Engine class to properly support max_iters as an alternative to max_epochs for controlling training duration.
Key changes include:
- Enhanced Engine state dict validation to support both
max_epochsandmax_iterstermination modes - Updated serialization logic to include appropriate termination parameters based on the training configuration
- Extended test coverage for various max_iters scenarios including resuming, state loading, and edge cases
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tests/ignite/engine/test_max_iters_fix.py |
Comprehensive test suite for max_iters functionality covering state dict operations, resuming, and validation |
tests/ignite/engine/test_engine_state_dict.py |
Updated existing tests to account for additional state dict keys |
tests/ignite/conftest.py |
Added defensive checks for pytest configuration options |
tests/ignite/base/test_mixins_update.py |
Tests for updated Serializable mixin validation logic |
ignite/engine/engine.py |
Core Engine implementation with max_iters support and improved state management |
ignite/base/mixins.py |
Enhanced Serializable base class with grouped optional key validation |
| Various workflow and metric files | Minor fixes and updates |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ignite/metrics/vision/object_detection_average_precision_recall.py
Outdated
Show resolved
Hide resolved
goanpeca
force-pushed
the
fix/max-iters
branch
4 times, most recently
from
075f57c to
eba390f
Compare
vfdev-5
reviewed
Sep 1, 2025
Collaborator
vfdev-5
left a comment
vfdev-5
left a comment
There was a problem hiding this comment.
Thanks a lot for the PR @goanpeca !
I made the first pass and left comments essentially to split this PR into multiple ones and explain certain changes.
I'll check in more depth the max_iters related changes a bit later whether it works as expected.
Anyway, great job!
ignite/metrics/vision/object_detection_average_precision_recall.py
Outdated
Show resolved
Hide resolved
goanpeca
force-pushed
the
fix/max-iters
branch
9 times, most recently
from
fc1fcb5 to
cfd0d13
Compare
vfdev-5
reviewed
Sep 5, 2025
goanpeca
force-pushed
the
fix/max-iters
branch
from
cfd0d13 to
697ba81
Compare
goanpeca
force-pushed
the
fix/max-iters
branch
from
697ba81 to
243afad
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1521
Depends on: