Add noct_sam cell temperature model to PVSystem, ModelChain #1195
Conversation
pvlib/pvsystem.py
Outdated
|
|
||
| Returns | ||
| ------- | ||
| numeric or tuple of numeric |
pvlib/pvsystem.py
Outdated
|
|
||
| def _build_kwargs_noct_sam(array): | ||
| temp_model_kwargs = _build_kwargs([ | ||
| 'noct', 'eta_m_ref', 'transmittance_absorptance', |
There was a problem hiding this comment.
noct and eta_m_ref are required parameters, not keyword arguments with defaults, so we need to perform explicit lookups for them. User should see a KeyError if they're not found in each array.temperature_model_parameters.
There was a problem hiding this comment.
Added explicit calls and a ValueError
| 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.
#1196 will need to update for this one too
pvlib/modelchain.py
Outdated
| arg_list = [poa, temp_air, wind_speed] | ||
| if model == self.system.noct_sam_celltemp: | ||
| arg_list += [self.results.effective_irradiance] | ||
| self.results.cell_temperature = model(*tuple(arg_list)) |
There was a problem hiding this comment.
| arg_list = [poa, temp_air, wind_speed] | |
| if model == self.system.noct_sam_celltemp: | |
| arg_list += [self.results.effective_irradiance] | |
| self.results.cell_temperature = model(*tuple(arg_list)) | |
| kwargs = {} | |
| if model == self.system.noct_sam_celltemp: | |
| kwargs['effective_irradiance'] = self.results.effective_irradiance | |
| self.results.cell_temperature = model(poa, temp_air, wind_speed, **kwargs) |
There was a problem hiding this comment.
I like to pass keyword arguments as keyword arguments, but I don't feel strongly about it so feel free to reject.
pvlib/pvsystem.py
Outdated
| 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. |
There was a problem hiding this comment.
| effective_irradiance : numeric, tuple of numeric or None. | |
| effective_irradiance : numeric, tuple of numeric, or None. |
| 'array_height', 'mount_standoff'], | ||
| array.temperature_model_parameters) | ||
| try: | ||
| temp_model_kwargs['noct'] = \ |
There was a problem hiding this comment.
| temp_model_kwargs['noct'] = \ | |
| # noct_sam required args. | |
| # bundled with kwargs for simplicity | |
| temp_model_kwargs['noct'] = \ |
pvlib/pvsystem.py
Outdated
| temp_model_kwargs['eta_m_ref'] = \ | ||
| array.temperature_model_parameters['eta_m_ref'] | ||
| except KeyError: | ||
| msg = ('Parameter noct and eta_m_ref are required.' |
There was a problem hiding this comment.
| msg = ('Parameter noct and eta_m_ref are required.' | |
| msg = ('Parameters noct and eta_m_ref are required.' |
| arg_list = [poa, temp_air, wind_speed] | ||
| kwargs = {} | ||
| if model == self.system.noct_sam_celltemp: | ||
| kwargs['effective_irradiance'] = self.results.effective_irradiance | ||
| self.results.cell_temperature = model(*tuple(arg_list)) |
There was a problem hiding this comment.
@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
self.results.cell_temperature = model(poa, temp_air, wind_speed, **kwargs)if you want to keep arg_list, this would also work:
self.results.cell_temperature = model(*arg_list, **kwargs)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.
My mistake, I'll fix it.
docs/sphinx/source/api.rstfor 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`).Continuation of #1177