Skip to content

Python translation of NREL SPA code #48

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

Merged
merged 2 commits into from
Apr 14, 2015
Merged

Conversation

alorenzo175
Copy link
Contributor

Since the NREL SPA C code isn't included in the repo anymore, I thought I would translate the algorithm to python. The translation produces the same results as the C code. When using numpy arrays and operations, this code is just as fast as the cythonized C code. When using numba, the import can take a while because it has to compile, but it is much faster for subsequent calls. Numba can also release the GIL and can use multiple threads.

I also made some changes to the spa C cython files and spa function to take pressure and temperature inputs (instead of hard coded values) and also output apparent zenith and elevation.

This isn't quite ready for a merge yet; I still want to get delta_t automatically from the USNO instead of having a set value or making the user set it.

In [21]:

print len(times)
%time npy = spa_python(times, loc, pressure=82000, temperature=11, delta_t=67.0, how='numpy')
DEBUG:pvlib:Calculating solar position with spa_python code
DEBUG:pvlib:tz_convert to UTC
DEBUG:pvlib:Reloading spa module without compiling
1441
CPU times: user 51 ms, sys: 3 ms, total: 54 ms
Wall time: 52.3 ms
In [22]:

print len(times)
%time nba = spa_python(times, loc, pressure=82000, temperature=11, delta_t=67.0, how='numba', numthreads=1)
DEBUG:pvlib:Calculating solar position with spa_python code
DEBUG:pvlib:tz_convert to UTC
DEBUG:pvlib:Reloading spa module, compiling with numba
DEBUG:pvlib:Only using one thread for calculation
1441
CPU times: user 3.72 s, sys: 16 ms, total: 3.74 s
Wall time: 3.74 s
In [23]:

print len(times)
%time nba = spa_python(times, loc, pressure=82000, temperature=11, delta_t=67.0, how='numba', numthreads=1)
DEBUG:pvlib:Calculating solar position with spa_python code
DEBUG:pvlib:tz_convert to UTC
DEBUG:pvlib:Only using one thread for calculation
1441
CPU times: user 34 ms, sys: 2 ms, total: 36 ms
Wall time: 35 ms
In [24]:

print len(times)
%time nba = spa_python(times, loc, pressure=82000, temperature=11, delta_t=67.0, how='numba', numthreads=4)
DEBUG:pvlib:Calculating solar position with spa_python code
DEBUG:pvlib:tz_convert to UTC
1441
CPU times: user 39 ms, sys: 1 ms, total: 40 ms
Wall time: 12.4 ms
In [25]:

print len(times)
%time eph = get_solarposition(times, loc, 'pyephem', pressure=82000, temperature=11)
DEBUG:pvlib:using PyEphem to calculate solar position
DEBUG:pvlib:tz_convert to UTC
1441
CPU times: user 155 ms, sys: 1 ms, total: 156 ms
Wall time: 154 ms
In [27]:

%time spac = get_solarposition(times, loc, 'spa_c', pressure=82000, temperature=11)
DEBUG:pvlib:using built-in spa code to calculate solar position
DEBUG:pvlib:tz_convert to UTC
CPU times: user 49 ms, sys: 3 ms, total: 52 ms
Wall time: 49.8 ms

@wholmgren
Copy link
Member

This is very cool and very well done. To be clear, @alorenzo175 used NREL's paper, not their code, to make this.

@wholmgren
Copy link
Member

Would have liked to comment on the diff, but my computer/browser chokes on long diffs...

On get_solarposition, I suggest that:

  • 'spa' is removed, or it stays but as the default and performs a try: spa_numba; except: spa_python.
  • If 'spa' is removed, 'spa_python' is the default.
  • 'spa_c' becomes 'spa_nrel_c'.
  • maybe add **kwargs to this and pass to worker functions?

I also suggest that the function spa becomes spa_nrel_c.

Would like to hear others input on the above (cc @Calama-Consulting @bmu).

In spa_python:

  • First sentence in the docstring implies that this is modeled on the NREL C code instead of the NREL paper.
  • atmos_refrac is not in the docstring.
  • The how block needs a few comments for why it uses this logic and how it works with the other modules. I think I understand after studying the code, but import logic and decorators is definitely not beginner Python.
  • how docstring is not consistent with the end of the how block.

@bmu
Copy link
Contributor

bmu commented Apr 6, 2015

This is great!! (I tried to do this myself a while ago, but I gave up after a while!)

One question:
A lot of the spa code is on time calculations. Do you think it is possible to separate the time calculations and the calculation of the sun position? I think (but I'm not sure), that true solar time is called apparent sideral time in the code (the code uses naming conventions of astronomers, and I am not an expert in this field ;-). see #47

@alorenzo175
Copy link
Contributor Author

'spa' is removed, or it stays but as the default and performs a try: spa_numba; except: spa_python.
If 'spa' is removed, 'spa_python' is the default.

I would recommend that spa_numpy is the default. Numba can take a few seconds to compile and the numpy code is pretty fast. If someone is doing a lot of calculations and needs the speed, they can specify the use of numba and many threads.

'spa_c' becomes 'spa_nrel_c'.

I thought changing 'spa' to something else, but I didn't want to break the API. I think we should all agree on if/how to change it.

maybe add **kwargs to this and pass to worker functions?

Good idea.

@alorenzo175
Copy link
Contributor Author

One question:
A lot of the spa code is on time calculations. Do you think it is possible to separate the time calculations and the calculation of the sun position? I think (but I'm not sure), that true solar time is called apparent sideral time in the code (the code uses naming conventions of astronomers, and I am not an expert in this field ;-). see #47

It is definitely possible to separate out the time calculations. I'll play around with it a little to see the best way to do it.

@alorenzo175 alorenzo175 force-pushed the numba-spa branch 2 times, most recently from 52a428b to 64cc8d5 Compare April 8, 2015 23:44
@alorenzo175 alorenzo175 closed this Apr 8, 2015
@alorenzo175 alorenzo175 reopened this Apr 8, 2015
@alorenzo175
Copy link
Contributor Author

I added a equation_of_time output from the solar position algorithm. I think any conversion to true solar time should be another function in solarposition.

I also renamed spa to spa_c, made the numpy calculation the default method to remove the pyephem dependency, and added a function to get sunrise, sunset, and sun transit time.

I think it is ready to merge as soon as it is reviewed.

@wholmgren
Copy link
Member

This looks good to me. @bmu or @Calama-Consulting?

@bmu
Copy link
Contributor

bmu commented Apr 10, 2015

I don't see, were equation_of_time is returned and the code is to complex at a first look, so I cannot say I have understood everything. However I would merge it in its present form.

@wholmgren
Copy link
Member

If anybody wants something changed further then they can make a new issue for it.

wholmgren added a commit that referenced this pull request Apr 14, 2015
Python translation of NREL SPA code
@wholmgren wholmgren merged commit cc6a962 into pvlib:master Apr 14, 2015
@uvchik
Copy link
Contributor

uvchik commented Apr 15, 2015

There are no errors but unused imports (import warnings, import pytz) and unused variables e using the following expression: except ImportError as e but the e is not used anymore.

Furthermore there some PEP8 issues but I don't know how strict we wanna be.

@uvchik
Copy link
Contributor

uvchik commented Apr 16, 2015

@alorenzo175
Do you want to add this or shall I create a new pull request about these issues.

@alorenzo175
Copy link
Contributor Author

I can change it quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants