-
Notifications
You must be signed in to change notification settings - Fork 102
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
compute_presence #340
Comments
Oops, good catch - the I also agree on picking on presence reporting standard - though the string outputs are now consistent between model types, there's an inconsistency between the string report using % and the plot using ratio (see fe.report output here), which is no good in terms of consistency. If we prefer the % then the quick fix is to update the event presence plot to use percent - does that sound good? We could also optionally also simplify the output types of |
On the math side of things 0.80 is better than 80% though. But 80% is more easily human-understandable. So go with 80% on plotting outputs. |
Event model object strings and plot outputs are now consolidated on reporting presence in % - so I think that addresses this fully now! |
The
compute_presence
function allows users to pass whether they want the output as a ration or percent, but theSpectralTimeModel
report from strings.py always prints the %, regardless of theoutput
flag.So we either need to change the report, or just pick one presence reporting style and stick with it.
I'm inclined to vote for the latter, since "80%" is less ambiguous than (0.80), people can divide by 100 if they really want, and fewer options to track is better if they're not critical for the method or understanding.
The text was updated successfully, but these errors were encountered: