fix(core/lifelong): correct rounds condition to fix HAS_COMPLETED_INITIAL_TRAINING in standard mode#573
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: g-k-s-03 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
…TIAL_TRAINING in standard mode, Issue 572 Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request adds validation to ensure the aggregation module is present and initialized in federated learning, raising a ValueError if not. It also modifies the condition for checking if initial training has completed in lifelong learning from rounds < 1 to rounds <= 1. However, this change to rounds <= 1 introduces a regression in no-inference and hard-example-mining modes where the first round starts at r=0. A suggestion is provided to conditionally determine the initial training round based on the active mode.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if rounds <= 1: | ||
| os.environ["HAS_COMPLETED_INITIAL_TRAINING"] = 'False' | ||
| else: | ||
| os.environ["HAS_COMPLETED_INITIAL_TRAINING"] = 'True' |
There was a problem hiding this comment.
Changing the condition to rounds <= 1 fixes the issue in standard mode (where the first round starts at r=1), but it introduces a regression in no-inference and hard-example-mining modes. In those modes, the first round starts at r=0 and the second round is r=1. With rounds <= 1, the second round (r=1) will be incorrectly treated as the initial training round (HAS_COMPLETED_INITIAL_TRAINING set to 'False').
To support both loop starting points correctly, we should determine whether it is the initial training round based on the active mode.
| if rounds <= 1: | |
| os.environ["HAS_COMPLETED_INITIAL_TRAINING"] = 'False' | |
| else: | |
| os.environ["HAS_COMPLETED_INITIAL_TRAINING"] = 'True' | |
| mode = (self.model_eval_config or {}).get('model_metric', {}).get('mode') | |
| is_initial = rounds < 1 if mode in ('no-inference', 'hard-example-mining') else rounds <= 1 | |
| if is_initial: | |
| os.environ['HAS_COMPLETED_INITIAL_TRAINING'] = 'False' | |
| else: | |
| os.environ['HAS_COMPLETED_INITIAL_TRAINING'] = 'True' |
There was a problem hiding this comment.
Valid point — updated the fix to be mode-aware. The condition now uses rounds < 1 for no-inference and hard-example-mining modes (preserving original behavior) and rounds <= 1 for standard mode (the actual fix). This avoids the regression you identified
9adcc5f to
f042092
Compare
…L_TRAINING Signed-off-by: g-k-s-03 <govindsingh97704@gmail.com>
|
@jaypume The PR description above reflects the initial approach. The actual fix is mode-aware it uses rounds < 1 for no-inference and hard-example-mining modes (preserving original behaviour) and rounds <= 1 for standard mode. See the latest commit for the final implementation |
|
/assign |
Description
In standard benchmark mode (
elif mode != 'multi-inference'), thetraining loop starts at
r=1. The_train()method usesrounds < 1to determine whether initial training has completed. Since
1 < 1isalways
False,HAS_COMPLETED_INITIAL_TRAININGis set to'True'on the very first call — before any training has occurred.
This is a silent correctness bug. Every lifelong learning algorithm
receives an incorrect signal on the first training round, causing it
to either attempt loading a non-existent checkpoint or skip
knowledge-base initialization. Benchmark metrics (BWT, FWT, Matrix)
are silently corrupted as a result.
The no-inference and hard-example-mining modes are unaffected — they
pass
r=0, which still satisfiesrounds <= 1.Root Cause
The condition
rounds < 1only evaluates toTruewhenr=0. Thestandard mode loop starts at
r=1, so the condition is neverTrueon the first call. Changing
<to<=makes the conditionTruewhen
r=1(first call in standard mode) andFalsefor allsubsequent rounds (
r=2, 3, ...).Fix
Files Changed
core/testcasecontroller/algorithm/paradigm/lifelong_learning/lifelong_learning.pyTesting
The fix can be verified by adding the following assertion to any
lifelong learning basemodel and running a standard mode benchmark:
This assertion passes after the fix and fails on the unfixed code.
Fixes #572