Skip to content

Add StrongStationarityProperty#257

Merged
PierreQuinton merged 51 commits intomainfrom
stationarity_property
Mar 27, 2025
Merged

Add StrongStationarityProperty#257
PierreQuinton merged 51 commits intomainfrom
stationarity_property

Conversation

@PierreQuinton
Copy link
Copy Markdown
Contributor

@PierreQuinton PierreQuinton commented Feb 24, 2025

  • Change matrix generation in _inputs.py to be able to generate strongly stationary matrices, strictly weakly stationary matrices and non weakly-stationary matrices
  • Change typical matrices to contain the newly added types of matrices
  • Rename a few things in _inputs.py to make lines shorter
  • Increase tolerance of some existing tests (LinearUnderScalingProperty and test_equivalence_mean of CAGrad) to make them pass on the new typical_matrices
  • Add StrongStationarityProperty in _property_testers.py
  • Make Constant, DualProj, Mean, Random, Sum and UPGrad extend StrongStationarityProperty.

Note that MGDA should not and does not pass these tests. CAGrad should pass these tests but does not (probably due to some implementation issue of CAGrad). AlignedMTL passes the tests but it's hard to tell if it should in theory. For a few other aggregators, we did not try the property tester, and we do not know if they have the property in theory.
EDIT: this might have changed after changing the dimensions of the matrices that we test.

@PierreQuinton PierreQuinton added cc: test Conventional commit type for changes to tests. package: aggregation labels Feb 24, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Base automatically changed from improve_test_matrices to main March 20, 2025 11:02
Comment thread tests/unit/aggregation/_property_testers.py Outdated
@ValerianRey ValerianRey force-pushed the stationarity_property branch from aba36d8 to e7e1c29 Compare March 23, 2025 11:37
@ValerianRey ValerianRey force-pushed the stationarity_property branch from e7e1c29 to 4f31b23 Compare March 23, 2025 11:41
@ValerianRey
Copy link
Copy Markdown
Contributor

On CUDA, 2 non-stationarity tests fail for UPGrad and 1 for MGDA. On CPU, everything passes. Specifically, this assertion:

assert norm > 1e-03

fails, because the norm is 2.2e-4 and 5.5e-5 for 2 matrices with UPGrad, and 8.9e-9 for one matrix with MGDA.

@ValerianRey
Copy link
Copy Markdown
Contributor

Docstrings of all three new classes could be improved IMO.

@ValerianRey
Copy link
Copy Markdown
Contributor

Shouldn't many more aggregators have this property? You told me a few months ago that those aggregators were supposed to be strongly stationary:
✅ mean
❌ MGDA
✅ Dualproj
🤷🏻‍♂️ PCGrad
🤷🏻‍♂️ GradDrop
✅ IMTL-G
❌ CAGrad
✅ RGW
✅ NashMTL

@ValerianRey

This comment was marked as resolved.

@ValerianRey ValerianRey marked this pull request as ready for review March 23, 2025 11:57
@PierreQuinton
Copy link
Copy Markdown
Contributor Author

I'll go over all aggregator theoretically to assess this property, I think from memory that you are correct

@PierreQuinton
Copy link
Copy Markdown
Contributor Author

PierreQuinton commented Mar 25, 2025

An aggregator is said to be:

  • Weakly stationary if it returns $0$ on and only on matrices that are weak stationary.
  • Strongly stationary if it returns $0$ on and only on matrices that are strongly stationary.

Note that some aggregators may be neither. Typically, when there is a pref vector, for a positive pref vector we can have strong, for a non-negative pref vector we can only have weak and for an arbitrary pref vector we have no guarantee.

Weak stationary aggregators:

Strong stationary aggregators:

Neither:

Unkown:

For ConFIG, the behavior is not specified on (any) stationary matrices, so it is very implementation dependent, testing should therefore be enough.

@ValerianRey
Copy link
Copy Markdown
Contributor

  • Maybe we should entirely drop the concept of weak stationarity: it's not interesting enough to justify having a property for this.
  • Then, I would define Strongly stationary to be: "if it returns 0 only on matrices that are strongly stationary." (without the "on and"). The reason is that the converse of this (the "on and") is implied by the non-conflicting property.

@PierreQuinton PierreQuinton changed the title Implements the stationarity properties Add StrongStationarityProperty Mar 27, 2025
@PierreQuinton PierreQuinton merged commit f227121 into main Mar 27, 2025
14 checks passed
@PierreQuinton PierreQuinton deleted the stationarity_property branch March 27, 2025 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cc: test Conventional commit type for changes to tests. package: aggregation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants