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

added percent bias score (pbias) #639

Merged
merged 8 commits into from
Sep 5, 2024
Merged

added percent bias score (pbias) #639

merged 8 commits into from
Sep 5, 2024

Conversation

durgals
Copy link
Contributor

@durgals durgals commented Aug 22, 2024

Please work through the following checklists. Delete anything that isn't relevant.

Development for new xarray-based metrics

  • Works with n-dimensional data and includes reduce_dims, preserve_dims, and weights args.
  • Typehints added
  • Docstrings complete and followed Napoleon (google) style
  • Reference to a webpage is in docstring
  • Imported into the API

Testing of new xarray-based metrics

  • 100% unit test coverage
  • Test that metric is compatible with dask.
  • Test that metrics work with inputs that contain NaNs
  • Test that broadcasting with xarray works
  • Test both reduce and preserve dims arguments work
  • Test that errors are raised as expected
  • Test that it works with both xr.Dataarrays and xr.Datasets

Tutorial notebook

  • added a short introduction to why you would use that metric and what it tells you on existing Additive_and_multiplicative_bias.ipynb

Documentation

@tennlee

This comment was marked as resolved.

Steph-Chong

This comment was marked as resolved.

@tennlee

This comment was marked as resolved.

@tennlee

This comment was marked as resolved.

@tennlee

This comment was marked as resolved.

@durgals
Copy link
Contributor Author

durgals commented Aug 26, 2024

@Steph-Chong

Am I correct this metric is also known as "Percent Bias"? Are there are any other names by which the metric is known?
Yes, this metric is commonly known as 'Percent Bias'. It is also referred to as 'Relative Mean Error' when not multiplied by 100.

nicholasloveday

This comment was marked as resolved.

@tennlee
Copy link
Collaborator

tennlee commented Aug 29, 2024

Hi @durgals I think we have checked this pretty thoroughly now. Please go ahead and make the requested changes, and then I'll give it another review at that point.

@durgals

This comment was marked as resolved.

@tennlee

This comment was marked as resolved.

@durgals

This comment was marked as resolved.

@tennlee
Copy link
Collaborator

tennlee commented Sep 3, 2024

I see you have made some updates. Does that address everything so far? I wasn't sure whether to wait for more or whether that completes the update. Feel free to go through and respond to or resolve the messages on GitHub where they have now been dealt with. If those updates address everything, I'll go through and do a final check to see if this can now be merged.

@tennlee
Copy link
Collaborator

tennlee commented Sep 4, 2024

@durgals can you please also add yourself to the .zenodo.json file if you would like to be recognised as an author in Zenodo.

(edit - adding some more info on how to do this)

We recently starting archiving scores releases on zenodo, see https://doi.org/10.5281/zenodo.12697241. Would you like to listed as a “creator” the next time scores is archived on zenodo?

If so, please add your details at the bottom of the “creators” section in the .zenodo.json. The fields you will need to complete are:

  1. orcid (optional)
  2. affiliation
  3. Name (last name, given names)

@durgals
Copy link
Contributor Author

durgals commented Sep 5, 2024

I see you have made some updates. Does that address everything so far? I wasn't sure whether to wait for more or whether that completes the update. Feel free to go through and respond to or resolve the messages on GitHub where they have now been dealt with. If those updates address everything, I'll go through and do a final check to see if this can now be merged.

I have updated addressing most or all of the above comments.

@durgals
Copy link
Contributor Author

durgals commented Sep 5, 2024

@durgals can you please also add yourself to the .zenodo.json file if you would like to be recognised as an author in Zenodo.

(edit - adding some more info on how to do this)

We recently starting archiving scores releases on zenodo, see https://doi.org/10.5281/zenodo.12697241. Would you like to listed as a “creator” the next time scores is archived on zenodo?

If so, please add your details at the bottom of the “creators” section in the .zenodo.json. The fields you will need to complete are:

  1. orcid (optional)
  2. affiliation
  3. Name (last name, given names)

Thanks. I will update .zenodo.json and commit with any other changes after final review.

@tennlee tennlee merged commit af98ce6 into nci:develop Sep 5, 2024
11 checks passed
@tennlee
Copy link
Collaborator

tennlee commented Sep 5, 2024

@durgals This all looks great to me! I have gone ahead and merged the PR. Great job, and thanks for doing high-quality work! Can you please raise a new PR to add yourself to the .zenodo.json file (don't try to resurrect this one or re-use this branch, that won't work smoothly)

@tennlee tennlee changed the title added additive bias percentage score (#613-pbias) added percent bias score (pbias) Sep 5, 2024
@durgals durgals deleted the 613-pbias branch September 6, 2024 06:39
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.

4 participants