Add scale_mode options to AlignedMTL#526
Add scale_mode options to AlignedMTL#526rkhosrowshahi wants to merge 4 commits intoSimplexLab:mainfrom
Conversation
Expose median and rmse scaling modes for the balance transformation to match original behavior while keeping min as default.
|
Local tests passed: . CI appears blocked pending workflow approval—please approve when convenient. |
|
All unit tests passed following CONTRIBUTING.md guidelines: Results: 2435 passed, 63 skipped, 33 xfailed CI workflows are currently awaiting maintainer approval before they can run. |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
ValerianRey
left a comment
ValerianRey
left a comment
There was a problem hiding this comment.
Hi! This looks really nice, and I think it can be merged as long as a few things are fixed.
Where is the official implementation located? It seems that the link to it (https://github.com/SamsungLabs/MTL) is now broken. I'm not sure why I used min instead of the default. Maybe the original implementation has changed in the meantime?
| elif scale_mode == "median": | ||
| scale = torch.median(lambda_) | ||
| elif scale_mode == "rmse": | ||
| scale = lambda_.mean() |
There was a problem hiding this comment.
Could you add code coverage for this? I think it should be enough to just parametrize test_expected_structure and test_permutation_invariant (from tests/unit/aggregation/test_aligned_mtl.py) with AlignedMTL(scale_mode="median") and AlignedMTL(scale_mode="rmse") (on top of the already existing AlignedMTL()).
| else: | ||
| raise ValueError( | ||
| f"Invalid scale_mode={scale_mode!r}. Expected 'min', 'median', or 'rmse'." | ||
| ) |
There was a problem hiding this comment.
Also need coverage for this. A simple test should be enough:
from pytest import raises
from utils.tensors import ones_
def test_invalid_scale_mode():
aggregator = AlignedMTL(scale_mode="test")
matrix = ones_(3, 4)
with raises(ValueError):
aggregator(matrix)| def __init__( | ||
| self, | ||
| pref_vector: Tensor | None = None, | ||
| scale_mode: Literal["min", "median", "rmse"] = "min", |
There was a problem hiding this comment.
Could you add a changelog entry (under "Unreleased") saying that it's now possible to provide a scale_mode parameter for AlignedMTL and AlignedMTLWeighting?
Hi, the original repo is no longer accessible, but based on my memory, they have implemented three scale modes: min, median, and RMSE. I think they reported min, but min is significantly noisy in many-objective optimization. I found this repo still using the old implementation, which I use as reference: |
Add coverage for median/rmse modes and invalid scale_mode handling.
Document the new scale_mode parameter for AlignedMTL and AlignedMTLWeighting.
for more information, see https://pre-commit.ci
|
Addressed review feedback:
Unit tests: UV_NO_SYNC=1 uv run pytest tests/unit (2604 passed, 63 skipped, 33 xfailed). |
2 participants
Summary
Test plan
pytest tests/unit/aggregation/test_aligned_mtl.py tests/unit/aggregation/test_values.py