pingswept / pysolar

Pysolar is a collection of Python libraries for simulating the irradiation of any point on earth by the sun. It includes code for extremely precise ephemeris calculations.
http://pysolar.org
GNU General Public License v3.0
373 stars 125 forks source link

Make get_elevation and get_azimuth numpy-aware #74

Closed fsteinmetz closed 6 years ago

fsteinmetz commented 6 years ago

This is a first step towards #72, that I implemented for my needs. Introduced the module numeric.py to include common functions between math and numpy, and some tools to help in the vectorization. numpy is only an optional dependency, which allows to pass arrays of latitudes and longitudes to get_elevation and get_azimuth. However, times still can not be passed as arrays because some functions in solartime.py are not very straightforward to vectorize. This approach could be extended to more functions in pysolar.

Let me know what you think of this approach.

pingswept commented 6 years ago

This looks great! Thanks, François!

louis-red commented 6 years ago

Good job ! This looks neat ! But true, I happened myself more often than not, to call repeatedly get_azimuth on fixed latitude and longitude, but arrays of datetimes. One way to do it would maybe to consider that if you want to use numpy, you should have pandas too ; and pandas define a great deal of vectorized operations on series of datetimes, grouped under the pandas.Series.dt property A good starting point could be to first vectorize get_altitude/azimuth_fast, as they do not require functions from solartime I personally do not understand the intricacies of the solartime module enough to know if all its functions could be vectorizable.

fsteinmetz commented 6 years ago

Thanks for the encouraging words :) It would indeed be nice to push it forward. I'm not sure about how you would use pandas, it is possible to get yday, hours, etc from arrays of datetime64 with just numpy. Example:

def tm_yday_numpy(d):
    dd = numpy.array(d, dtype='datetime64[D]')
    dy = numpy.array(d, dtype='datetime64[Y]')
    return (dd - dy).astype('int') + 1

The most problematic seem to be get_leap_second and get_delta_t, which will require a bit of thinking for vectorization.

And following your advice I tried to vectorize the *fast versions of get_altitude and get_azimuth, it went fine for the first but get_azimuth_fast actually calls get_altitude (which requires functions from solartime.py) :-/

fsteinmetz commented 6 years ago

So it seems that we could just switch to using get_altitude_fast instead of get_altitude in get_azimuth_fast... If you agree with that, I will submit another PR.

pingswept commented 6 years ago

Yes, using get_altitude_fast instead of get_altitude in get_azimuth_fast is the right solution. The whole point of the *_fast functions is to make a crappy approximation quickly.

fsteinmetz commented 6 years ago

Ok, thanks. I just issued a PR for that. By the way, is there a description somewhere of how "crappy" (and why) the *_fast functions are ?

pingswept commented 6 years ago

PR accepted. I don't think there is any documentation on the existence of the *_fast functions. I originally wrote them as an easy way to double check the rest of the library as I was testing it.

I would love to see charts added to https://github.com/pingswept/pysolar/blob/master/test/validation.ipynb (and subsequently http://docs.pysolar.org/en/latest/#validation) that compared the results and execution times of the various functions, if you or anyone else wants to do that.