Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement CRPS for normal distribution. #60

Merged
merged 11 commits into from
Oct 1, 2023

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Sep 26, 2023

  • Have an erf function. Thinking about upstreaming it to somewhere like raft-legate. Not sure if legateboost wants to depend on raft-legate.
  • Use lambda for type dispatching.
  • Some code copied from xgboost for handling ndarray in c++. We can flatten the array in Python for this particular function, but it would be nice to have indexing support in c++.

The metric is important for probabilistic forecasting.

Copy link
Contributor

@RAMitchell RAMitchell left a comment

Choose a reason for hiding this comment

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

I think the native code is a bit too complicated for this project. What about one of these in pure cunumeric: https://en.wikipedia.org/wiki/Error_function#Approximation_with_elementary_functions?

I think you could probably do something in 5 lines or so. It may not be that fast, but it might be better to implement it as simply as possible, then adopt a possible legate scipy implementation later.

src/CMakeLists.txt Outdated Show resolved Hide resolved
@trivialfis
Copy link
Member Author

All comments are addressed.

Copy link
Contributor

@RAMitchell RAMitchell left a comment

Choose a reason for hiding this comment

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

LGTM. Trying to get CI back up and running today, then we can merge.

PROPERTIES
CXX_STANDARD 17
CXX_STANDARD_REQUIRED ON
POSITION_INDEPENDENT_CODE ON)
Copy link
Contributor

Choose a reason for hiding this comment

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

What is position independant code for?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nothing, it's the default, just a habit of being explicit. Can revert if needed.

@RAMitchell RAMitchell merged commit 998b9ea into rapidsai:main Oct 1, 2023
4 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.

2 participants