feat(aggregation): Add getters and setters to all remaining aggregators#660
Merged
ValerianRey merged 15 commits intomainfrom Apr 21, 2026
Merged
feat(aggregation): Add getters and setters to all remaining aggregators#660ValerianRey merged 15 commits intomainfrom
ValerianRey merged 15 commits intomainfrom
Conversation
Expose pref_vector, norm_eps, and reg_eps as properties on DualProj and DualProjWeighting, mirroring the UPGrad pattern. The norm_eps and reg_eps setters validate that the new value is non-negative.
Expose pref_vector and scale_mode as properties on AlignedMTL and AlignedMTLWeighting, mirroring the UPGrad pattern.
Expose pref_vector as a property on ConFIG. The setter rebuilds the internal weighting via pref_vector_to_weighting to keep state in sync.
Expose c and norm_eps as properties on CAGrad and CAGradWeighting. The setters validate that the new value is non-negative.
Expose n_byzantine and n_selected as properties on Krum and KrumWeighting. The setters enforce n_byzantine >= 0 and n_selected >= 1.
Expose epsilon and max_iters as properties on MGDA and MGDAWeighting. The setters enforce that both parameters are strictly positive.
Expose n_tasks, max_norm, update_weights_every, and optim_niter as properties on NashMTL and _NashMTLWeighting. Validation: n_tasks/update_weights_every/optim_niter > 0, max_norm >= 0 (0 disables norm clipping, matching existing forward logic). Setters do not automatically reset the internal state; users must call reset() if the state needs rebuilding (especially after changing n_tasks). Documented in both class docstrings.
Expose trim_number as a property on TrimmedMean. The setter preserves the existing non-negative validation.
Expose leak as a property on GradDrop. The setter preserves the existing 1D-tensor validation. f is kept as a plain public attribute (no validation needed), matching the pattern used for UPGrad's solver.
Covers AlignedMTL, CAGrad, ConFIG, DualProj, GradDrop, Krum, MGDA, NashMTL, and TrimmedMean.
ValerianRey
added
cc: feat
ValerianRey
approved these changes
Apr 21, 2026
Contributor
ValerianRey
left a comment
ValerianRey
left a comment
There was a problem hiding this comment.
Very good, thank you very much! Just a few nitpicks in the tests that I'll fix myself before merging.
@PierreQuinton I think we could release v0.11.0 with this.
Co-authored-by: Valérian Rey <31951177+ValerianRey@users.noreply.github.com>
Contributor
Author
|
@ValerianRey I meant to review myself before setting ready to review but you were faster than me, thanks! |
Contributor
No worries! That was a really good PR ty! |
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
2 participants
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.
Summary
Replicates the getter/setter pattern established in #658 (UPGrad) across every other aggregator that has constructor parameters. For each, the Weighting class stores each parameter as
_namewith a public@property+@setterpair; setters validate their inputs; outer aggregator classes forward to the inner weighting when applicable;__init__calls the setters so validation always runs;__repr__/__str__read through the properties.Commits are split per aggregator for easier review:
pref_vector,norm_eps,reg_eps(direct copy of UPGrad pattern)pref_vector,scale_modepref_vector(single class, no Weighting)c,norm_epsn_byzantine,n_selectedepsilon,max_itersn_tasks,max_norm,update_weights_every,optim_nitertrim_numberleak(fkept as a plain attribute, matching UPGrad'ssolver)Validation choices
pref_vectorsetters rebuild the internalweightingviapref_vector_to_weighting(mirrors UPGrad).norm_eps,reg_eps,c,n_byzantine,trim_number,max_norm.max_norm == 0disables the clipping, matching the existingif self.max_norm > 0check inNashMTL.forward.epsilon,max_iters,n_selected,n_tasks,update_weights_every,optim_niter.leaksetter preserves the existing 1-D-tensor validation.scale_modehas no validation (relies on theSUPPORTED_SCALE_MODEtype alias, as requested).NashMTL note
NashMTL is stateful. Setters do not auto-reset internal state — users must call
.reset()after changing parameters (especiallyn_tasks, which determines the shape of the cached state). This is documented in both docstrings.Test plan
uv run pytest tests/unit/aggregation→ 1804 passeduv run ty check src/torchjd/aggregation→ all checks passed