Skip to content

[Fix] Poor performance with the NegativeBinomial DistributionLoss#1289

Merged
marcopeix merged 1 commit intoNixtla:mainfrom
JQGoh:issue/712-neg-binomial
Mar 18, 2025
Merged

[Fix] Poor performance with the NegativeBinomial DistributionLoss#1289
marcopeix merged 1 commit intoNixtla:mainfrom
JQGoh:issue/712-neg-binomial

Conversation

@JQGoh
Copy link
Contributor

@JQGoh JQGoh commented Mar 16, 2025

An attempt to address #712 , suspect that the wrong scaling parameters were chosen during the Negative Binomial distribution computation

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@JQGoh JQGoh changed the title [Bug] Poor performance with the NegativeBinomial DistributionLoss [Fix] Poor performance with the NegativeBinomial DistributionLoss Mar 16, 2025
@JQGoh
Copy link
Contributor Author

JQGoh commented Mar 16, 2025

@Antoine-Schwartz When you have time, could you help to check out the branch issue/712-neg-binomial from https://github.com/JQGoh/neuralforecast/tree/issue/712-neg-binomial and test whether the proposed fix works for you?

These two files are my tests using your reproducible script (only consider NHITS model due to limited computing resource) and we can see the 'on-dev-branch-evaluate.csv' has improved performance than `on-master-branch-evaluate.csv' (without the proposed fix).
on-dev-branch-evalute.csv
on-master-branch-evalute.csv

@elephaint @marcopeix @jmoralez
Appreciate your review on this too. Per my understanding, the scaling behavior is similar to effect introduced for normal_scale_decouple

@Antoine-Schwartz
Copy link

Antoine-Schwartz commented Mar 17, 2025

Hello @JQGoh,

I'm glad someone is finally taking this subject seriously. Indeed, as I didn't have the time to dissect the code in detail to understand the problem, I had gone for the default StudentT in my codes.
Did you try to relaunch the example of my issue (it runs fast too) with your fix? I don't think I'll have time to test it this week, but I'll let you know as soon as I can.

However, my team has noticed that v3.0.0 of neuralforecast has a negative impact on the overall performance of TFT (but maybe not NHITS?). We'll be opening an issue on this subject this week. If this is actually the case, it will make comparisons difficult: TFT v2.0.1 with StudentT vs. TFT v3.0.1 (including your fix) with NegativeBinomial.

@JQGoh
Copy link
Contributor Author

JQGoh commented Mar 17, 2025

Hello @JQGoh,

I'm glad someone is finally taking this subject seriously. Indeed, as I didn't have the time to dissect the code in detail to understand the problem, I had gone for the default StudentT in my codes.
Did you try to relaunch the example of my issue (it runs fast too) with your fix? I don't think I'll have time to test it this week, but I'll let you know as soon as I can.

However, my team has noticed that v3.0.0 of neuralforecast has a negative impact on the overall performance of TFT (but maybe not NHITS?). We'll be opening an issue on this subject this week. If this is actually the case, it will make comparisons difficult: TFT v2.0.1 with StudentT vs. TFT v3.0.1 (including your fix) with NegativeBinomial.

I relaunched your example with one model only, NHITS. The attached file 'on-dev-branch-evalute.csv' demonstrates improvement in the loss score as compared to another file.

But I would like to hear from others and invite more tests for this. Checking whether this indeed solves the issue. When I read the equation and consider the transformation on the parameters used for the mean-parametrization of negative binomial distribution, the fix makes sense too.

@JQGoh
Copy link
Contributor Author

JQGoh commented Mar 17, 2025

Hello @JQGoh,

I'm glad someone is finally taking this subject seriously. Indeed, as I didn't have the time to dissect the code in detail to understand the problem, I had gone for the default StudentT in my codes.
Did you try to relaunch the example of my issue (it runs fast too) with your fix? I don't think I'll have time to test it this week, but I'll let you know as soon as I can.

However, my team has noticed that v3.0.0 of neuralforecast has a negative impact on the overall performance of TFT (but maybe not NHITS?). We'll be opening an issue on this subject this week. If this is actually the case, it will make comparisons difficult: TFT v2.0.1 with StudentT vs. TFT v3.0.1 (including your fix) with NegativeBinomial.

Appreciate your help if you could take whichever model/dataset, but you try with two versions of runs

  • The dev branch with my fix
  • The latest installed Neuralforecast library

And compare the results

@Antoine-Schwartz
Copy link

I took a few minutes to test it out.
Your fix seems to work correctly from v3.0.0! Performance is similar to other distributions, if not better, as when using DeepAR.

@marcopeix
Copy link
Contributor

Also tested on my end, and I see an improvement with DeepAR, NHITS and TFT. Thanks a lot @JQGoh for the fix, and thank you @Antoine-Schwartz for testing!

@marcopeix marcopeix marked this pull request as ready for review March 18, 2025 15:12
@marcopeix marcopeix self-requested a review March 18, 2025 15:12
@marcopeix marcopeix linked an issue Mar 18, 2025 that may be closed by this pull request
@marcopeix marcopeix merged commit e2f473a into Nixtla:main Mar 18, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Loss] Poor performance with the NegativeBinomial DistributionLoss

3 participants