bancaditalia / black-it

Black-box abm calibration kit by the Bank of Italy
https://bancaditalia.github.io/black-it/
GNU Affero General Public License v3.0
43 stars 2 forks source link

xgboost: add clipping of loss values to the float32 limits #35

Closed AldoGl closed 1 year ago

AldoGl commented 1 year ago

Proposed changes

Internally the XGBoost routine operates a conversion to float32, and this fails if the absolute values of the loss are exceedingly large. This commit adds a simple clipping operation before the loss values are passed to the XGBoost package.

Types of changes

What types of changes does your code introduce? Put an x in the boxes that apply

Checklist

Put an x in the boxes that apply.

codecov-commenter commented 1 year ago

Codecov Report

Merging #35 (37ab3e7) into main (03da1df) will decrease coverage by 0.03%. The diff coverage is 94.11%.

Additional details and impacted files [![Impacted file tree graph](https://codecov.io/gh/bancaditalia/black-it/pull/35/graphs/tree.svg?width=650&height=150&src=pr&token=FwbdPgBFTD&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None)](https://codecov.io/gh/bancaditalia/black-it/pull/35?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) ```diff @@ Coverage Diff @@ ## main #35 +/- ## ========================================== - Coverage 97.21% 97.17% -0.04% ========================================== Files 29 29 Lines 1435 1452 +17 ========================================== + Hits 1395 1411 +16 - Misses 40 41 +1 ``` | [Impacted Files](https://codecov.io/gh/bancaditalia/black-it/pull/35?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [black\_it/samplers/xgboost.py](https://codecov.io/gh/bancaditalia/black-it/pull/35?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-YmxhY2tfaXQvc2FtcGxlcnMveGdib29zdC5weQ==) | `98.21% <94.11%> (-1.79%)` | :arrow_down: |
muxator commented 1 year ago
AldoGl commented 1 year ago
  • split the PR in two commits. The first one triggers the problem, the second one shows the fix. The final version is the same as the one submitted by @AldoGl.
  • some lint fixes

Thank you for your help @muxator, merging now!