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

BUG: Fix flux calculation on surface when there is no fluxing_field #4042

Merged
merged 3 commits into from Aug 1, 2022

Conversation

jzuhone
Copy link
Contributor

@jzuhone jzuhone commented Jul 30, 2022

PR Summary

Nominally, when one creates a YTSurface object using ds.surface, and then uses calculate_flux on the object to calculate a flux through the surface, if no fluxing_field is provided it should default to 1.0 dimensionless. I discovered by accident today that this isn't the case--mainly because np.ones_like now preserves units (see code below). This is the fix for that. The test has been improved to catch it.

PR Checklist

  • New features are documented, with docstrings and narrative docs
  • Adds a test for any bugs fixed. Adds tests for new features.

@jzuhone jzuhone added the bug label Jul 30, 2022
@neutrinoceros
Copy link
Member

neutrinoceros commented Jul 31, 2022

Looks good to me, just one question: do we know since when np.ones_like started preserving units ? I checked that there is no difference between unyt 2.8 and 2.9.2 (current), using numpy 1.22.3 (current)
There could be a lot of similar bugs, might be interesting to know what changed and when.

@matthewturk matthewturk merged commit b015877 into yt-project:main Aug 1, 2022
neutrinoceros pushed a commit to neutrinoceros/yt that referenced this pull request Aug 8, 2022
BUG: Fix flux calculation on surface when there is no fluxing_field
@neutrinoceros neutrinoceros added this to the 4.0.5 milestone Aug 9, 2022
@jzuhone jzuhone deleted the fix_surface branch November 7, 2022 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants