Warnings verbosity #1213
Warnings verbosity #1213
Conversation
Pull Request Test Coverage Report for Build 11976595227Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| @@ -1,6 +1,50 @@ | |||
| __all__ = ("CCLError", "CCLWarning", "CCLDeprecationWarning",) | |||
| __all__ = ("CCLError", "CCLWarning", "CCLDeprecationWarning", | |||
There was a problem hiding this comment.
Start reviewing here.
| @@ -40,8 +84,8 @@ def __hash__(self): | |||
|
|
|||
| @classmethod | |||
| def enable(cls): | |||
There was a problem hiding this comment.
Are the deprecation warnings covered by the verbosity filter as well?
There was a problem hiding this comment.
I don't think so, but let me check
There was a problem hiding this comment.
No they are not. Do you think they should?
There was a problem hiding this comment.
My gut feeling is yes - they're good to know while building a model but you probably don't want them in an MCMC log.
There was a problem hiding this comment.
If they mute them and CCL's deprecation cycle completes, their MCMC will simply stop working, and they won't be able to figure out why. I don't think it's a good idea muting those.
There was a problem hiding this comment.
Indeed, but deprecation warnings have to do with CCL functionality whereas CCLWarnings mostly have to do with numerical stability, so they represent different things. At the very least I'd add a different flag to separate the verbosity level for deprecation-specific warnings. BTW now that v3 is out, is CCLDeprecationWarning used at all?
There was a problem hiding this comment.
Oh, and also, deprecation warnings should be on my default, contrary to other, informative-only warnings, because that's how you tell people that the API will change.
There was a problem hiding this comment.
CCLDeprecationWarnings are not used right now. I'll leave them there for the future in any case.
There was a problem hiding this comment.
They seemed to be used here:
https://github.com/LSSTDESC/CCL/blob/master/pyccl/baryons/baccoemu_baryons.py#L221
and the decorators in
https://github.com/LSSTDESC/CCL/blob/master/pyccl/_core/deprecations.py
But the latter don't seem to be used.
There was a problem hiding this comment.
As developers we can decide what the priority of the different warnings are. If we decide that specific deprecation warnings are sufficiently important, we can just tag them as "high", and they'll go through the filter.
@tilmantroester I've now added the CCLDeprecationWarnings to the list of warnings that can be suppressed if desired.
|
We should document this somewhere though and make a point to check any model with |
CCL often throws warnings about things that are relatively benign and should not plague the output of e.g. MCMC runs. This PR implements a granular way to control the verbosity of the warning messages thrown by CCL.
It uses some code by @nikfilippas .
While doing this I fixed a couple of bugs in the documentation.
Most of the changes are cosmetic. Anyone reviewing should start by looking at the changes in
error.py(the rest is boilerplate)