Skip to content

[ENH] refactor _loss_mapping to tags of metrics, entirely#2327

Open
Faakhir30 wants to merge 8 commits into
sktime:mainfrom
Faakhir30:loss_mapping
Open

[ENH] refactor _loss_mapping to tags of metrics, entirely#2327
Faakhir30 wants to merge 8 commits into
sktime:mainfrom
Faakhir30:loss_mapping

Conversation

@Faakhir30

@Faakhir30 Faakhir30 commented Jun 29, 2026

Copy link
Copy Markdown
Member

Reference Issues/PRs

Closes #1957

What does this implement/fix? Explain your changes.

  • refactor _loss_mapping.py to tags
  • get compatible losses by filtering all_objects of type metric and specific metric_type based on pred_type and y_type
  • move test kwargs for losses to metric pkg level
  • introduced method get_test_train_params for this

PR checklist

  • The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.
  • Added/modified tests
  • Used pre-commit hooks when committing to ensure that code is compliant with hooks. Install hooks with pre-commit install.
    To run hooks independent of commit, execute pre-commit run --all-files

@Faakhir30

Faakhir30 commented Jun 29, 2026

Copy link
Copy Markdown
Member Author

only updated Deeper model and BetaDistributionLoss pkg for now.
TODO: Will update remaining metric pkgs and model pkgs, after approval of design by maintainers

cc @phoeenniixx @fkiraly

Edit: Done, ready for revview

Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
@Faakhir30 Faakhir30 marked this pull request as ready for review June 30, 2026 07:27
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.00000% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@4d8d97c). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...ributions_pkg/_mqf2/_mqf2_distribution_loss_pkg.py 50.00% 2 Missing ⚠️
pytorch_forecasting/tests/test_all_estimators.py 96.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2327   +/-   ##
=======================================
  Coverage        ?   87.53%           
=======================================
  Files           ?      166           
  Lines           ?     9767           
  Branches        ?        0           
=======================================
  Hits            ?     8550           
  Misses          ?     1217           
  Partials        ?        0           
Flag Coverage Δ
cpu 87.53% <94.00%> (?)
pytest 87.53% <94.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

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

for loss_pkg in compatible_loss_pkgs:
loss = loss_pkg.get_cls()()
loss_name = loss.__class__.__name__
loss_params = deepcopy(loss_pkg.get_default_params())

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add a way to pass some other "params" to losses here?
I think that would make it overcomplicated no?
What do you think @Faakhir30 @fkiraly ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a solution will be to have some method that has dict of losses: loss_kwargs at model pkg level and testing framework override for that particular model+loss combination.

Currently I'm unable to see any such case, we can add it later if any such case arrive.

@phoeenniixx phoeenniixx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!
I think it is ready! Just a small doubt and a suggestion (see above)
Please add docstrings to all public methods atleast - to prevent any confusions on what the methods do!

@phoeenniixx phoeenniixx added enhancement New feature or request module:test_framework ptf-v1 Related to `pytorch-forecasting` v1 ptf-v2 Related to `pytorch-forecasting` v2 labels Jul 2, 2026
Signed-off-by: Faakhir30 <zahidfaakhir@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request module:test_framework ptf-v1 Related to `pytorch-forecasting` v1 ptf-v2 Related to `pytorch-forecasting` v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ENH] refactor _loss_mapping to tags of metrics, entirely

3 participants