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

Fix mean radiant temperature segfaults #1315

Merged
merged 5 commits into from Mar 9, 2023

Conversation

Zeitsperre
Copy link
Collaborator

@Zeitsperre Zeitsperre commented Mar 8, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.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?

  • adjustments to tests to prevent test_mean_radiant_temperature() from causing segfaults

Does this PR introduce a breaking change?

No.

Other information:

There is something thread-unsafe when making multiple calls to atmos.mean_radiant_temperature. My theory is that dask is trying to read the exact same blocks of data in rapid succession, as the values between all calls are being compared at the same time. If we .load() this data, we don't trigger and this is fine, somehow.

@Zeitsperre Zeitsperre added the bug Something isn't working label Mar 8, 2023
@Zeitsperre Zeitsperre added this to the v0.42 milestone Mar 8, 2023
@Zeitsperre Zeitsperre self-assigned this Mar 8, 2023
@Zeitsperre
Copy link
Collaborator Author

Well, that was almost too easy...

Copy link
Collaborator

@aulemahal aulemahal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, let's see with all the tests.

@github-actions github-actions bot added the approved Approved for additional tests label Mar 9, 2023
@Zeitsperre Zeitsperre merged commit bef87c5 into master Mar 9, 2023
9 checks passed
@Zeitsperre Zeitsperre deleted the fix_mean_radiant_temperature branch March 9, 2023 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PyOpenSci] test_mean_radiant_temperature sometimes fails with segfault
2 participants