Skip to content

Conversation

@juliettelavoie
Copy link
Contributor

@juliettelavoie juliettelavoie commented Aug 5, 2025

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes #xyz
  • (If applicable) Documentation has been added / updated (for bug fixes / features).
  • (If applicable) Tests have been added.
  • CHANGELOG.rst has been updated (with summary of main changes).
    • Link to issue (:issue:number) and pull request (:pull:number) has been added.

What kind of change does this PR introduce?

  • Fix clip for 0. When we had a 0 in float32 and we clipped to the next float32, it would become a 0 (float 32) again in data_prime. If we cast it to float64, it could stay not exactly 0.
    • our tests did not catch this because the inputs were in 64.
    • The casting behavior is different for numpy <2.0, so I pin it above.
  • I have encountered climate models with hurs above 100... I have found that the data_request suggested to clip the data, but not everybody did it. (https://earthscience.stackexchange.com/questions/22850/why-are-the-cmip6-model-outputs-for-relative-humidity-not-scaled-to-between-1-10)
    • I am thinking maybe we can do the clipping ourselves, so I added that option with "permissive".

Does this PR introduce a breaking change?

yes, the behavior will be different for 0.

Other information:

@juliettelavoie juliettelavoie changed the title add permissive option and float 0 add permissive option and fix for 0 Aug 5, 2025
@juliettelavoie juliettelavoie marked this pull request as draft August 5, 2025 14:22
@coveralls
Copy link

coveralls commented Aug 5, 2025

Coverage Status

coverage: 78.173%. remained the same
when pulling 0cd5f1e on fix-clip0
into 251cd9c on main.

@juliettelavoie juliettelavoie marked this pull request as ready for review September 2, 2025 13:12
@Zeitsperre Zeitsperre added this to the v0.6.0 milestone Oct 14, 2025
@Zeitsperre Zeitsperre added bug Something isn't working enhancement New feature or request labels Oct 14, 2025
@juliettelavoie
Copy link
Contributor Author

The "permissive" option was necessary when clipping before the logit. I don't think we should clip anymore so this is kind of irrelevant now. But, I am waiting for people (genre Pierre et Martin) to approve the document on hurs before giving up completely on it.

In that case, I would still merge but only with the Fix clip for 0 part. so stand-by for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants