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 raising a unyt array to an array power in sensible cases #524

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

neutrinoceros
Copy link
Member

Trying to fix the few cases where it does make sense to raise a unyt array to a power array.
Close #522

@neutrinoceros neutrinoceros added the bug Something isn't working label Oct 8, 2024
@neutrinoceros neutrinoceros marked this pull request as ready for review October 8, 2024 13:51
@neutrinoceros neutrinoceros added this to the 3.0.4 milestone Oct 8, 2024
Copy link

@xshaokun xshaokun left a comment

Choose a reason for hiding this comment

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

I’m sorry for the delayed response. When

a = unyt_array([1, 2, 3])
p = unyt_array([1, 2, 3, 4])

a**p should not raise an error. However, I tried it and that’s not the case.

@pytest.mark.parametrize(
"p",
[
pytest.param(unyt_array([0, 1, 2, 3]), id="non-uniform power"),
Copy link

Choose a reason for hiding this comment

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

This should be valid only when a is a dimensionless array.

@chrishavlin
Copy link
Contributor

a**p should not raise an error. However, I tried it and that’s not the case.

What's your expectation when the array shapes don't match? In your example, those arrays will raise a broadcast error with plain numpy, so I wouldn't expect the operation to work with unyt.

a = np.array([1, 2, 3])
p = np.array([1, 2, 3, 4]
a**p
ValueError: operands could not be broadcast together with shapes (3,) (4,) 

@xshaokun
Copy link

xshaokun commented Nov 4, 2024

a**p should not raise an error. However, I tried it and that’s not the case.

What's your expectation when the array shapes don't match? In your example, those arrays will raise a broadcast error with plain numpy, so I wouldn't expect the operation to work with unyt.


a = np.array([1, 2, 3])

p = np.array([1, 2, 3, 4]

a**p

ValueError: operands could not be broadcast together with shapes (3,) (4,) 

Oh, I'm sorry for the typo! These two arrays are supposed to be the same, or any two dimensionless unyt_arrays with the same shape.

@neutrinoceros
Copy link
Member Author

Phew, that was a wild ride, but I think I fixed it. I don't like how complex this is turning out but I'm running out of time. Let me know if you see ways to simplify the logic.

@xshaokun
Copy link

xshaokun commented Nov 5, 2024

Thank you so much for your work, everything is working fine now. Since I don’t fully understand some of your implements, I’m unable to suggest ways to simplify the logic. Nevertheless, I really appreciate it!

@neutrinoceros
Copy link
Member Author

Alright. @jzuhone this is currently the last bugfix on the 3.0.4 milestone. Can you have a look ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

An error occurs when the exponent is an array
3 participants