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

Extend CRPS documentation #1

Open
frazane opened this issue Jul 28, 2023 · 5 comments
Open

Extend CRPS documentation #1

frazane opened this issue Jul 28, 2023 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@frazane
Copy link
Owner

frazane commented Jul 28, 2023

  • CRPS gif (area in between predictive cdf and observation cdf)
  • simple convergence experiment for estimators
@frazane frazane added the documentation Improvements or additions to documentation label Jul 28, 2023
@nicholasloveday
Copy link
Contributor

Additionally, it would be good to list the available estimators in the documentation. I think that crps_ensemble can take several.

I had to go digging through the code to find the available options here https://github.com/frazane/scoringrules/blob/main/scoringrules/core/crps/_gufuncs.py#L346

@nicholasloveday
Copy link
Contributor

nicholasloveday commented Aug 7, 2024

In the documentation it has

>>> from scoringrules import crps
>>> crps.ensemble(pred, obs)

This raises an error. Instead, I had to do

import scoringrules
scoringrules.crps_ensemble(obs, pred)

Note that the forecast and observed args are also out of order when comparing the function implementation to the existing docs and need to be swapped.

@nicholasloveday
Copy link
Contributor

With the typehints you have "ArrayLike" and "Array" as strings for the forecast and observed data types. This isn't particularly useful when looking at the docs to see what data types it can takes. E.g., https://github.com/frazane/scoringrules/blob/main/scoringrules/_crps.py#L11

Is there a reason why you don't use scoringrules.core.typing.Array and scoringrules.core.typing.ArrayLike in the type hints? I see that there is an if statement in the import so you may have a reason for it being this way and things may also get complicated with supporting multiple backends.

I had to go looking in https://github.com/frazane/scoringrules/blob/main/scoringrules/core/typing.py to see what data types it could handle. It would be nice if NDArray | JaxArray | torchTensor | tensorflowTensor etc appeared in the docs rather than "Array"

@nicholasloveday
Copy link
Contributor

The example for twCRPS for ensembles doesn't use the v_func arg which is a required function. It would be good to give an example of setting up and using this chaining function.

@frazane
Copy link
Owner Author

frazane commented Sep 3, 2024

With the typehints you have "ArrayLike" and "Array" as strings for the forecast and observed data types. This isn't particularly useful when looking at the docs to see what data types it can takes. E.g., https://github.com/frazane/scoringrules/blob/main/scoringrules/_crps.py#L11

Is there a reason why you don't use scoringrules.core.typing.Array and scoringrules.core.typing.ArrayLike in the type hints? I see that there is an if statement in the import so you may have a reason for it being this way and things may also get complicated with supporting multiple backends.

I had to go looking in https://github.com/frazane/scoringrules/blob/main/scoringrules/core/typing.py to see what data types it could handle. It would be nice if NDArray | JaxArray | torchTensor | tensorflowTensor etc appeared in the docs rather than "Array"

@nicholasloveday

Because of the multi-backend support, types are defined dynamically with typing.TypeVar, with the bound being the union of the supported array types. The reason they are not used directly is that the import statements related to types are not executed at runtime, but only by static type checkers (see https://peps.python.org/pep-0484/#runtime-or-type-checking), so as to not incur in ImportError if some backends are not installed. Maybe I should opt for try except statements during import?

When using modern IDEs like VSCode, these type hints defined as strings are handled like forward reference and are still "clickable" and give you the information you need. But it's true that it would be better if these also appeared in the documentation. I will definitely look into that. Thanks for the feedback!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants