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

Reassignment throws error in calculate_cos_solar_zenith_angle_integrated #27

Closed
jarataraj opened this issue Dec 31, 2022 · 4 comments
Closed

Comments

@jarataraj
Copy link

The Bug

While attempting to use calculate_cos_solar_zenith_angle_integrated via xr.apply_ufunc as in:

integralCzaEaHour = [xr.apply_ufunc(thermofeel.calculate_cos_solar_zenith_angle_integrated, ds.lat, ds.lon, time.dt.year, 
    time.dt.month, time.dt.day, time.dt.hour, 0, 1) for time in ds.time]

I get the following error:
ValueError: non-broadcastable output operand with shape (571,1) doesn't match the broadcast shape (571,1440)

The Solution

An answer to a related stack overflow question suggests eliminating the += operator, as in changing a += b to a = a + b. Sure enough, when I change line 316 in thermofeel.py from integral += w[n] * calculate_cos_solar_zenith_angle( to integral = integral + w[n] * calculate_cos_solar_zenith_angle(, everything works as expected. The stack overflow answer includes an explanation as to why.

@tlmquintino
Copy link
Member

@jarataraj many thanks for pointing out this issue.
I think it makes sense to apply the fix. Would you be able to provide a pull request with solution?

@jarataraj
Copy link
Author

@tlmquintino Will do! Is it ok for me to change all instances of a += b and similar to a = a + b and similar? Or should I just apply the fix to calculate_cos_solar_zenith_angle_integrated? I know I got the error for two different thermofeel functions but I can't remember what the other was and haven't yet tested all of them.

@tlmquintino
Copy link
Member

@jarataraj sorry for the late reply, I've been traveling.
Yes please do go ahead and I think it makes sense doing that substitution everywhere.

jarataraj added a commit to jarataraj/thermofeel that referenced this issue Jan 20, 2023
replace operations like `a += b` with `a = a + b`
see [issue ecmwf#27](ecmwf#27)
@tlmquintino tlmquintino linked a pull request Jun 5, 2024 that will close this issue
tlmquintino added a commit that referenced this issue Jun 5, 2024
@tlmquintino
Copy link
Member

this is now fixed as part of the code moved to another python module (earthkit-meteo) and the rest of the fixed was added directly to develop branch.

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 a pull request may close this issue.

2 participants