-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add noct_sam cell temperature model to PVSystem, ModelChain #1195
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
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
74800b0
add ModelChain, PVSystem methods
cwhanse 2fbd304
first cut at pvsystem tests
cwhanse c3175b0
fix iterable, called_once_with in test
cwhanse 64b16bf
derp
cwhanse e176da8
handling of optional argument, misspelling
cwhanse 5608f9a
mimic ModelChain tests to noct_sam
cwhanse 4a1dea3
add required params to test fixture
cwhanse 37ffb87
fix key value
cwhanse aab7d96
complete test spec
cwhanse 3b50f1a
complete test coverage, whatsnew
cwhanse 3616e82
docstring work
cwhanse dcb6fbc
additions to api
cwhanse 84a452c
docstring reverts, raise if missing arg
cwhanse 2664783
add types to method test
cwhanse 9613bb8
edits from review
cwhanse File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -143,7 +143,8 @@ class PVSystem: | |||||||||
Module parameters as defined by the SAPM, CEC, or other. | ||||||||||
|
||||||||||
temperature_model_parameters : None, dict or Series, default None. | ||||||||||
Temperature model parameters as defined by the SAPM, Pvsyst, or other. | ||||||||||
Temperature model parameters as required by one of the models in | ||||||||||
pvlib.temperature (excluding poa_global, temp_air and wind_speed). | ||||||||||
|
||||||||||
modules_per_string: int or float, default 1 | ||||||||||
See system topology discussion above. | ||||||||||
|
@@ -750,8 +751,6 @@ def fuentes_celltemp(self, poa_global, temp_air, wind_speed): | |||||||||
if you want to match the PVWatts behavior, you can override it by | ||||||||||
including a ``surface_tilt`` value in ``temperature_model_parameters``. | ||||||||||
|
||||||||||
Notes | ||||||||||
----- | ||||||||||
The `temp_air` and `wind_speed` parameters may be passed as tuples | ||||||||||
to provide different values for each Array in the system. If not | ||||||||||
passed as a tuple then the same value is used for input to each Array. | ||||||||||
|
@@ -781,6 +780,82 @@ def _build_kwargs_fuentes(array): | |||||||||
) | ||||||||||
) | ||||||||||
|
||||||||||
@_unwrap_single_value | ||||||||||
def noct_sam_celltemp(self, poa_global, temp_air, wind_speed, | ||||||||||
effective_irradiance=None): | ||||||||||
""" | ||||||||||
Use :py:func:`temperature.noct_sam` to calculate cell temperature. | ||||||||||
|
||||||||||
Parameters | ||||||||||
---------- | ||||||||||
poa_global : numeric or tuple of numeric | ||||||||||
Total incident irradiance in W/m^2. | ||||||||||
|
||||||||||
temp_air : numeric or tuple of numeric | ||||||||||
Ambient dry bulb temperature in degrees C. | ||||||||||
|
||||||||||
wind_speed : numeric or tuple of numeric | ||||||||||
Wind speed in m/s at a height of 10 meters. | ||||||||||
|
||||||||||
effective_irradiance : numeric, tuple of numeric, or None. | ||||||||||
The irradiance that is converted to photocurrent. If None, | ||||||||||
assumed equal to ``poa_global``. [W/m^2] | ||||||||||
|
||||||||||
Returns | ||||||||||
------- | ||||||||||
temperature_cell : numeric or tuple of numeric | ||||||||||
The modeled cell temperature [C] | ||||||||||
|
||||||||||
Notes | ||||||||||
----- | ||||||||||
The `temp_air` and `wind_speed` parameters may be passed as tuples | ||||||||||
to provide different values for each Array in the system. If not | ||||||||||
passed as a tuple then the same value is used for input to each Array. | ||||||||||
If passed as a tuple the length must be the same as the number of | ||||||||||
Arrays. | ||||||||||
""" | ||||||||||
# default to using the Array attribute, but allow user to | ||||||||||
# override with a custom surface_tilt value | ||||||||||
poa_global = self._validate_per_array(poa_global) | ||||||||||
temp_air = self._validate_per_array(temp_air, system_wide=True) | ||||||||||
wind_speed = self._validate_per_array(wind_speed, system_wide=True) | ||||||||||
|
||||||||||
# need effective_irradiance to be an iterable | ||||||||||
if effective_irradiance is None: | ||||||||||
effective_irradiance = tuple([None] * self.num_arrays) | ||||||||||
else: | ||||||||||
effective_irradiance = self._validate_per_array( | ||||||||||
effective_irradiance) | ||||||||||
|
||||||||||
def _build_kwargs_noct_sam(array): | ||||||||||
temp_model_kwargs = _build_kwargs([ | ||||||||||
'transmittance_absorptance', | ||||||||||
'array_height', 'mount_standoff'], | ||||||||||
array.temperature_model_parameters) | ||||||||||
try: | ||||||||||
# noct_sam required args | ||||||||||
# bundled with kwargs for simplicity | ||||||||||
temp_model_kwargs['noct'] = \ | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||
array.temperature_model_parameters['noct'] | ||||||||||
temp_model_kwargs['eta_m_ref'] = \ | ||||||||||
array.temperature_model_parameters['eta_m_ref'] | ||||||||||
except KeyError: | ||||||||||
msg = ('Parameters noct and eta_m_ref are required.' | ||||||||||
' Found {} in temperature_model_parameters.' | ||||||||||
.format(array.temperature_model_parameters)) | ||||||||||
raise KeyError(msg) | ||||||||||
return temp_model_kwargs | ||||||||||
return tuple( | ||||||||||
temperature.noct_sam( | ||||||||||
poa_global, temp_air, wind_speed, | ||||||||||
effective_irradiance=eff_irrad, | ||||||||||
**_build_kwargs_noct_sam(array)) | ||||||||||
for array, poa_global, temp_air, wind_speed, eff_irrad in zip( | ||||||||||
self.arrays, poa_global, temp_air, wind_speed, | ||||||||||
effective_irradiance | ||||||||||
) | ||||||||||
) | ||||||||||
|
||||||||||
@_unwrap_single_value | ||||||||||
def first_solar_spectral_loss(self, pw, airmass_absolute): | ||||||||||
|
||||||||||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -223,6 +223,18 @@ def pvwatts_dc_pvwatts_ac_fuentes_temp_system(): | |
return system | ||
|
||
|
||
@pytest.fixture(scope="function") | ||
def pvwatts_dc_pvwatts_ac_noct_sam_temp_system(): | ||
module_parameters = {'pdc0': 220, 'gamma_pdc': -0.003} | ||
temp_model_params = {'noct': 45, 'eta_m_ref': 0.2} | ||
inverter_parameters = {'pdc0': 220, 'eta_inv_nom': 0.95} | ||
system = PVSystem(surface_tilt=32.2, surface_azimuth=180, | ||
module_parameters=module_parameters, | ||
temperature_model_parameters=temp_model_params, | ||
inverter_parameters=inverter_parameters) | ||
return system | ||
|
||
|
||
@pytest.fixture(scope="function") | ||
def system_no_aoi(cec_module_cs5p_220m, sapm_temperature_cs5p_220m, | ||
cec_inverter_parameters): | ||
|
@@ -693,6 +705,23 @@ def test_run_model_with_weather_fuentes_temp(sapm_dc_snl_ac_system, location, | |
assert not mc.results.ac.empty | ||
|
||
|
||
def test_run_model_with_weather_noct_sam_temp(sapm_dc_snl_ac_system, location, | ||
weather, mocker): | ||
weather['wind_speed'] = 5 | ||
weather['temp_air'] = 10 | ||
sapm_dc_snl_ac_system.temperature_model_parameters = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #1196 will need to update for this one too |
||
'noct': 45, 'eta_m_ref': 0.2 | ||
} | ||
mc = ModelChain(sapm_dc_snl_ac_system, location) | ||
mc.temperature_model = 'noct_sam' | ||
m_noct_sam = mocker.spy(sapm_dc_snl_ac_system, 'noct_sam_celltemp') | ||
mc.run_model(weather) | ||
assert m_noct_sam.call_count == 1 | ||
assert_series_equal(m_noct_sam.call_args[0][1], weather['temp_air']) | ||
assert_series_equal(m_noct_sam.call_args[0][2], weather['wind_speed']) | ||
assert not mc.results.ac.empty | ||
|
||
|
||
def test_run_model_tracker(sapm_dc_snl_ac_system, location, weather, mocker): | ||
system = SingleAxisTracker( | ||
module_parameters=sapm_dc_snl_ac_system.module_parameters, | ||
|
@@ -907,7 +936,9 @@ def test__prepare_temperature_arrays_weather(sapm_dc_snl_ac_system_same_arrays, | |
({'u0': 25.0, 'u1': 6.84}, | ||
ModelChain.faiman_temp), | ||
({'noct_installed': 45}, | ||
ModelChain.fuentes_temp)]) | ||
ModelChain.fuentes_temp), | ||
({'noct': 45, 'eta_m_ref': 0.2}, | ||
ModelChain.noct_sam_temp)]) | ||
def test_temperature_models_arrays_multi_weather( | ||
temp_params, temp_model, | ||
sapm_dc_snl_ac_system_same_arrays, | ||
|
@@ -1256,16 +1287,19 @@ def test_infer_spectral_model(location, sapm_dc_snl_ac_system, | |
|
||
|
||
@pytest.mark.parametrize('temp_model', [ | ||
'sapm_temp', 'faiman_temp', 'pvsyst_temp', 'fuentes_temp']) | ||
'sapm_temp', 'faiman_temp', 'pvsyst_temp', 'fuentes_temp', | ||
'noct_sam_temp']) | ||
def test_infer_temp_model(location, sapm_dc_snl_ac_system, | ||
pvwatts_dc_pvwatts_ac_pvsyst_temp_system, | ||
pvwatts_dc_pvwatts_ac_faiman_temp_system, | ||
pvwatts_dc_pvwatts_ac_fuentes_temp_system, | ||
pvwatts_dc_pvwatts_ac_noct_sam_temp_system, | ||
temp_model): | ||
dc_systems = {'sapm_temp': sapm_dc_snl_ac_system, | ||
'pvsyst_temp': pvwatts_dc_pvwatts_ac_pvsyst_temp_system, | ||
'faiman_temp': pvwatts_dc_pvwatts_ac_faiman_temp_system, | ||
'fuentes_temp': pvwatts_dc_pvwatts_ac_fuentes_temp_system} | ||
'fuentes_temp': pvwatts_dc_pvwatts_ac_fuentes_temp_system, | ||
'noct_sam_temp': pvwatts_dc_pvwatts_ac_noct_sam_temp_system} | ||
system = dc_systems[temp_model] | ||
mc = ModelChain(system, location, aoi_model='physical', | ||
spectral_model='no_loss') | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@cwhanse when attempting to merge your changes into #1197 I realized that this is not correct because kwargs is not passed to the function. I had suggested
if you want to keep
arg_list
, this would also work:note there is no need to cast the list to a tuple.
When fixing, we should add a test to ensure this value is passed properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My mistake, I'll fix it.