Add Muneer transposition model #2184
Conversation
still not tested
pvlib/irradiance.py
Outdated
| .. [1] Muneer, T., 1990, Solar radiation model for Europe. | ||
| Building services engineering research and technology, 11: 153-163. | ||
|
|
||
| .. [2] Moon P and Spencer D E Illumination from a non-uniform sky | ||
| Trans. Illum. Eng. Soc. (London) 37 707-725 (1942) |
There was a problem hiding this comment.
These two sources follow different citation convention.
Also, could you add a DOI?
There was a problem hiding this comment.
Resolved in 8b13595 . First time I do it, @AdamRJensen can you check if it is alright?
There was a problem hiding this comment.
@BernatNicolau both look typed out correctly, but I don't think the second DOI value is correct (review comment)
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Adam initial review Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Adam initial review Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Adam initial review Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Adam initial review Co-authored-by: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
Co-Authored-By: Adam R. Jensen <39184289+AdamRJensen@users.noreply.github.com>
There was a problem hiding this comment.
Looks really good! Some minor documentation comments down below.
Since this is a new feature, we have to do some additional changes to the code:
- Docs. I suppose you have tried to find your function at the built readthedocs page, but had no luck. That happens because each function we want to publicity show in the API needs to be listed in some index pages. The appropriate list for this function is in docs/sphinx/source/reference/irradiance/transposition.rst
- Tests. We want to ensure this function works as expected in the future, so tests is a must-have. Look for
test_irradiance.pyand try to add a test. You can just mimic current code or read more aboutpytest. If you need some help with that let us know. By the way, the best way I have found to make test data is by using a spreadsheet, so you can double check you implementation.
At some point a whatsnew entry will need to be made. Best is to defer that for a bit so we can have more reviews till then.
pvlib/irradiance.py
Outdated
| Surface tilt angle in decimal degrees. Tilt must be >=0 and | ||
| <=180. The tilt angle is defined as degrees from horizontal | ||
| (e.g. surface facing up = 0, surface facing horizon = 90) |
There was a problem hiding this comment.
| Surface tilt angle in decimal degrees. Tilt must be >=0 and | |
| <=180. The tilt angle is defined as degrees from horizontal | |
| (e.g. surface facing up = 0, surface facing horizon = 90) | |
| Surface tilt angle from horizontal. E.g., | |
| facing up = 0°, facing horizon = 90°. | |
| In degrees [°]. |
There was a problem hiding this comment.
Got the wording from perez. For consistency, should other functions also be updated or shall I keep the one that currently is (from perez)?
There was a problem hiding this comment.
echedey-ls edits are an improvement but I'm not in favor of changing documentation for other functions. You can change it here, but I think it's not a big deal if we use the old language.
|
It is quite hard to do the testing of the code since PVGIS directly provides the transposed data, without providing GHI, DHI and DNI (if I am wrong please tell me). I wanted to cross check if I obtained the same results by downloading the transposed signal and by applying it with the function, but I think I am reaching a dead end here... |
Not really! Most of the time we don't have original data to work with. It's a good enough test if you can make up some data by using a spreadsheet like Excel and implementing the model there. Just rewriting the equations and making input data up for DHI and b values, then typing both the inputs and expected output in the test should be fine. That way we can confirm this new code complies with the expected behaviour. |
It may be possible to recover the original GHI/DHI/DNI components by specifying |
|
I have come to realize that the Muneer transposition is not as straightforward as I thought. At the beginning I was following this paper which I understand is the original Muneer transposition. However, I have found this article that describes a Muneer model, referencing this book from 2004 which is more complex, but can surely be implemented. To be able to code the 2004 Muneer model, a I am not sure which model should pvlib offer (1990 or 2004). I would gladly accept some guidance here :) |
I suggest implementing the version from the book. PS. Somewhere I have a floppy disk containing the software from the book, in case it is needed. |
This could be a good standalone feature to be included! |
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
|
@BernatNicolau I would choose one (my preference is also 2004) and name it |
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
still needs testing
After having read the book section, I finally understood that the model is the same as 1990 but explained with greater detail and with some examples that compare different transposition models. So as far as I understand, there is no need to name it Note that I still need to do testing and add better documentation. |
muneer included in get_sky_diffuse code and documentation muneer included in get_total_irradiance documentation muneer function updated TODO: tests, create documentation
added documentation, tests and modifications according to flake8
still quite new to the flake8. May not pass the test
Co-authored-by: Cliff Hansen <cwhanse@sandia.gov>
Co-authored-by: RDaxini <143435106+RDaxini@users.noreply.github.com>
|
After looking into this some more, it seems to me that we have to regard the PVGIS/Hofierka model as being related to but distinct from the Muneer 1990 model. Compared with Muneer, Hofierka uses different equations for different conditions ( So, we need to decide which of the two models we are aiming to implement here. If we want to implement Muneer, we cannot use PVGIS to produce validation data for the tests. @AdamRJensen, this was your feature request -- any thoughts on how we should proceed? |
|
I agree with you @kandersolar . However, either way I don't think we can use PVGIS for validation, I was recently doing more testing and I am obtaining very spurious results while analyzing poa from PVGIS. I modified your testing code to produce the 12 months of the TMY for that example location: Click to show codeimport pvlib
import pandas as pd
import matplotlib.pyplot as plt
import numpy as np
import pvlib.tools as tools
surface_tilt = 20
surface_azimuth = 180
latitude = 40
longitude = -80
# get PVGIS GHI, DHI for one month by requesting the TMY dataset:
pvgis_tmy, months, _, meta = pvlib.iotools.get_pvgis_tmy(latitude, longitude)
pvgis_all = pd.DataFrame()
for y in range(1, 13):
print(y)
pvgis_tmy_year = pvgis_tmy.loc[pvgis_tmy.index.month == y]
# now get PVGIS POA for the same year/month:
year = months[y-1]['year'] # the year for the TMY january
pvgis_poa, _, meta = pvlib.iotools.get_pvgis_hourly(
latitude, longitude, start=year, end=year, surface_tilt=surface_tilt, surface_azimuth=surface_azimuth, usehorizon=False)
pvgis_poa = pvgis_poa.loc[pvgis_poa.index.month == y] # keep only january
# transpose GHI, DHI to POA:
sp = pvlib.solarposition.get_solarposition(
pvgis_poa.index, latitude, longitude)
sp.index = pvgis_poa.index
dni_extra = pvlib.irradiance.get_extra_radiation(pvgis_poa.index)
poa_diffuse_muneer = pvlib.irradiance.muneer(
surface_tilt, surface_azimuth, pvgis_tmy_year['dhi'], pvgis_tmy_year['ghi'], dni_extra, solar_azimuth=sp['azimuth'], solar_zenith=sp['zenith'], b=0)
poa_muneer = pvlib.irradiance.get_total_irradiance(
surface_tilt=surface_tilt,
surface_azimuth=surface_azimuth,
solar_zenith=sp['zenith'],
solar_azimuth=sp['azimuth'],
dni=pvgis_tmy_year['dni'],
ghi=pvgis_tmy_year['ghi'],
dhi=pvgis_tmy_year['dhi'],
dni_extra=dni_extra,
model="muneer",
)
aux = pd.DataFrame({
'sky_diff-PVGIS': pvgis_poa['poa_sky_diffuse'],
'sky_diff-muneer': poa_diffuse_muneer,
'poa-PVGIS': pvgis_poa.iloc[:, 0:2].sum(axis=1),
'poa-muneer': poa_muneer['poa_global']
})
out = pd.concat([aux, pvgis_tmy_year, sp, dni_extra], axis=1)
pvgis_all = pd.concat([pvgis_all, out])
pvgis_all.to_csv('comparison_allyear.csv')There are some months of the TMY where the poa value seems correct, but there are other months that the poa doesn't make any sense to me. Let me illustrate: On this day, the poa has a lower value than the ghi, and it seems to be a non-cloudy day. This day doesn't make sense to me either... |
|
I thought I had commented on the |
|
Just a comment, this discussion illustrates the original motivation for pvlib - to clarify what each is in each model. Hang in there @BernatNicolau |
|
I have created the function based on the 2004 book, but I have not pushed it to this PR because I don't know how to proceed:
This article 10.2790/91554 explains the transposition model used in PVGIS which basically is the one from the book with an improvement for low solar elevations. The function I have created is based on this article. |
|
I would say the more models, the |
Outside of PVGIS I have never heard of the Muneer transposition model, nor have I seen studies where it has an outstanding performance. Given that there are so many transposition models, I think we best only include the more recent Muneer model that is used by PVGIS. How well does it match PVGIS btw? I have made my own implementation and can get it to match quite well for most times. I'd be happy to collaborate on investigating how well it matches pvgis. |
Muneer from book adaptation
I have commited the muneer function from the book ( import pvlib
import pandas as pd
surface_tilt = 20
surface_azimuth = 180
latitude = 37
longitude = -4
# get PVGIS GHI, DHI for one month by requesting the TMY dataset:
pvgis_tmy, months, _, meta = pvlib.iotools.get_pvgis_tmy(
latitude, longitude, usehorizon=False)
pvgis_all = pd.DataFrame()
for y in range(1, 13):
print(y)
pvgis_tmy_year = pvgis_tmy.loc[pvgis_tmy.index.month == y]
# now get PVGIS POA for the same year/month:
year = months[y-1]['year'] # the year for the TMY january
pvgis_poa, _, meta = pvlib.iotools.get_pvgis_hourly(
latitude, longitude, start=year, end=year, surface_tilt=surface_tilt, surface_azimuth=surface_azimuth, usehorizon=False)
pvgis_poa = pvgis_poa.resample('H').mean()
pvgis_poa = pvgis_poa.loc[pvgis_poa.index.month == y] # keep only january
# transpose GHI, DHI to POA:
sp = pvlib.solarposition.get_solarposition(
pvgis_poa.index, latitude, longitude)
sp.index = pvgis_poa.index
dni_extra = pvlib.irradiance.get_extra_radiation(pvgis_poa.index)
poa_diffuse_muneer = pvlib.irradiance.muneer2004(
surface_tilt, surface_azimuth, pvgis_tmy_year['dhi'], pvgis_tmy_year['ghi'], dni_extra, solar_azimuth=sp['azimuth'], solar_zenith=sp['zenith'])
poa_muneer = pvlib.irradiance.get_total_irradiance(
surface_tilt=surface_tilt,
surface_azimuth=surface_azimuth,
solar_zenith=sp['zenith'],
solar_azimuth=sp['azimuth'],
dni=pvgis_tmy_year['dni'],
ghi=pvgis_tmy_year['ghi'],
dhi=pvgis_tmy_year['dhi'],
dni_extra=dni_extra,
model="muneer",
)
aux = pd.DataFrame({
'sky_diff-PVGIS': pvgis_poa['poa_sky_diffuse'],
'sky_diff-muneer': poa_diffuse_muneer,
'poa-PVGIS': pvgis_poa.iloc[:, 0:2].sum(axis=1),
'poa-muneer': poa_muneer['poa_global']
})
out = pd.concat([aux, pvgis_tmy_year, sp, dni_extra], axis=1)
pvgis_all = pd.concat([pvgis_all, out])
pvgis_all.to_csv('comparison_allyear.csv') |
|
Just wondering what might be the next step on this...? |
|
Moving to 11.3 (comment), hopefully we can get this merged in the next release:) |
|
Hi folks! I have been quite disconnected from contributing these days due to overload of tasks but I am willing to keep working on this :) Which do you think is the best way forward with this topic? @AdamRJensen Can you share your implementations/comparisons with PVGIS so we can collaborate on this? |
docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.