Skip to content

Deprecate/raise error for PVSystem "pass-through" properties #1196

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.

Sign up for GitHub

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 16 commits into from
May 20, 2021
Merged

Deprecate/raise error for PVSystem "pass-through" properties #1196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
1ee16e1
add and apply decorator
kandersolar Mar 13, 2021
4d27feb
fix everything that broke
kandersolar Mar 13, 2021
f5494aa
whatsnew
kandersolar Mar 13, 2021
9852c16
remove unnecessary fixture, on the correct branch this time
kandersolar Mar 28, 2021
49635f4
Update pvlib/modelchain.py
kandersolar May 12, 2021
7588a81
reflow docstring
kandersolar May 12, 2021
1824bd6
more docstring updates
kandersolar May 12, 2021
8fba8a5
Merge remote-tracking branch 'upstream/master' into passthrough
kandersolar May 19, 2021
01099ed
alphabetize
kandersolar May 19, 2021
a29acae
rephrase error message
kandersolar May 19, 2021
c775bc0
pvsystem.rst edits
kandersolar May 19, 2021
7b96395
create missing PVSystem setters
kandersolar May 19, 2021
71b07da
rephrase error message
kandersolar May 19, 2021
53ea45d
another pvsystem.rst edit
kandersolar May 19, 2021
4619a1e
fix coverage drop
kandersolar May 19, 2021
62a549e
add missing test
kandersolar May 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions docs/sphinx/source/pvsystem.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ default value may be overridden by specifying the `temp_ref` key in the

.. ipython:: python

system.module_parameters['temp_ref'] = 0
system.arrays[0].module_parameters['temp_ref'] = 0
# lower temp_ref should lead to lower DC power than calculated above
pdc = system.pvwatts_dc(1000, 30)
print(pdc)
Expand All @@ -124,7 +124,7 @@ passed to `PVSystem.module_parameters`:
inverter_parameters = {'pdc0': 5000, 'eta_inv_nom': 0.96}
system = pvsystem.PVSystem(module_parameters=module_parameters,
inverter_parameters=inverter_parameters)
print(system.module_parameters)
print(system.arrays[0].module_parameters)
print(system.inverter_parameters)


Expand All @@ -142,12 +142,9 @@ provided for each array, and the arrays are provided to
array_two = pvsystem.Array(module_parameters=module_parameters)
system_two_arrays = pvsystem.PVSystem(arrays=[array_one, array_two],
inverter_parameters=inverter_parameters)
print(system_two_arrays.module_parameters)
print([array.module_parameters for array in system_two_arrays.arrays])
print(system_two_arrays.inverter_parameters)

Note that in the case of a PV system with multiple arrays, the
:py:class:`~pvlib.pvsystem.PVSystem` attribute `module_parameters` contains
a tuple with the `module_parameters` for each array.

The :py:class:`~pvlib.pvsystem.Array` class includes those
:py:class:`~pvlib.pvsystem.PVSystem` attributes that may vary from array
Expand Down Expand Up @@ -188,7 +185,8 @@ these parameters can be specified using the `PVSystem.surface_tilt` and

# single south-facing array at 20 deg tilt
system_one_array = pvsystem.PVSystem(surface_tilt=20, surface_azimuth=180)
print(system_one_array.surface_tilt, system_one_array.surface_azimuth)
print(system_one_array.arrays[0].surface_tilt,
system_one_array.arrays[0].surface_azimuth)


In the case of a PV system with several arrays, the parameters are specified
Expand All @@ -201,8 +199,7 @@ for each array using the attributes `Array.surface_tilt` and `Array.surface_azim
array_two = pvsystem.Array(surface_tilt=30, surface_azimuth=220)
system = pvsystem.PVSystem(arrays=[array_one, array_two])
system.num_arrays
system.surface_tilt
system.surface_azimuth
[(array.surface_tilt, array.surface_azimuth) for array in system.arrays]


The `surface_tilt` and `surface_azimuth` attributes are used in PVSystem
Expand All @@ -218,7 +215,8 @@ and `solar_azimuth` as arguments.

# single south-facing array at 20 deg tilt
system_one_array = pvsystem.PVSystem(surface_tilt=20, surface_azimuth=180)
print(system_one_array.surface_tilt, system_one_array.surface_azimuth)
print(system_one_array.arrays[0].surface_tilt,
system_one_array.arrays[0].surface_azimuth)

# call get_aoi with solar_zenith, solar_azimuth
aoi = system_one_array.get_aoi(solar_zenith=30, solar_azimuth=180)
Expand Down
16 changes: 16 additions & 0 deletions docs/sphinx/source/whatsnew/v0.9.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,22 @@ Deprecations
* The ``eta_m`` parameter for :py:func:`~pvlib.temperature.pvsyst_cell` is
replaced by parameter ``module_efficiency``. (:issue:`1188`, :pull:`1218`)

* The following attributes of :py:class:`pvlib.pvsystem.PVSystem` and
:py:class:`pvlib.tracking.SingleAxisTracker` have been deprecated in
favor of the corresponding :py:class:`pvlib.pvsystem.Array` attributes:

* ``PVSystem.albedo``
* ``PVSystem.module``
* ``PVSystem.module_parameters``
* ``PVSystem.module_type``
* ``PVSystem.modules_per_string``
* ``PVSystem.racking_model``
* ``PVSystem.strings_per_inverter``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the alphabetization of the list preceding this one and would prefer to see that here too.

* ``PVSystem.surface_tilt``
* ``PVSystem.surface_azimuth``
* ``PVSystem.temperature_model_parameters``


Enhancements
~~~~~~~~~~~~
* Add :func:`~pvlib.iotools.read_bsrn` for reading BSRN solar radiation data
Expand Down
82 changes: 48 additions & 34 deletions pvlib/modelchain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,29 +408,32 @@ class ModelChain:
Passed to location.get_airmass.

dc_model: None, str, or function, default None
If None, the model will be inferred from the contents of
system.module_parameters. Valid strings are 'sapm',
'desoto', 'cec', 'pvsyst', 'pvwatts'. The ModelChain instance will
be passed as the first argument to a user-defined function.
If None, the model will be inferred from the parameters that
are common to all of system.arrays[i].module_parameters.
Valid strings are 'sapm', 'desoto', 'cec', 'pvsyst', 'pvwatts'.
The ModelChain instance will be passed as the first argument
to a user-defined function.

ac_model: None, str, or function, default None
If None, the model will be inferred from the contents of
system.inverter_parameters and system.module_parameters. Valid
strings are 'sandia', 'adr', 'pvwatts'. The
If None, the model will be inferred from the parameters that
are common to all of system.inverter_parameters.
Valid strings are 'sandia', 'adr', 'pvwatts'. The
ModelChain instance will be passed as the first argument to a
user-defined function.

aoi_model: None, str, or function, default None
If None, the model will be inferred from the contents of
system.module_parameters. Valid strings are 'physical',
'ashrae', 'sapm', 'martin_ruiz', 'no_loss'. The ModelChain instance
will be passed as the first argument to a user-defined function.
If None, the model will be inferred from the parameters that
are common to all of system.arrays[i].module_parameters.
Valid strings are 'physical', 'ashrae', 'sapm', 'martin_ruiz',
'no_loss'. The ModelChain instance will be passed as the
first argument to a user-defined function.

spectral_model: None, str, or function, default None
If None, the model will be inferred from the contents of
system.module_parameters. Valid strings are 'sapm',
'first_solar', 'no_loss'. The ModelChain instance will be passed
as the first argument to a user-defined function.
If None, the model will be inferred from the parameters that
are common to all of system.arrays[i].module_parameters.
Valid strings are 'sapm', 'first_solar', 'no_loss'.
The ModelChain instance will be passed as the first argument to
a user-defined function.

temperature_model: None, str or function, default None
Valid strings are: 'sapm', 'pvsyst', 'faiman', 'fuentes', 'noct_sam'.
Expand Down Expand Up @@ -691,9 +694,10 @@ def dc_model(self, model):
model = model.lower()
if model in _DC_MODEL_PARAMS.keys():
# validate module parameters
module_parameters = tuple(
array.module_parameters for array in self.system.arrays)
missing_params = (
_DC_MODEL_PARAMS[model] -
_common_keys(self.system.module_parameters))
_DC_MODEL_PARAMS[model] - _common_keys(module_parameters))
if missing_params: # some parameters are not in module.keys()
raise ValueError(model + ' selected for the DC model but '
'one or more Arrays are missing '
Expand All @@ -716,7 +720,8 @@ def dc_model(self, model):

def infer_dc_model(self):
"""Infer DC power model from Array module parameters."""
params = _common_keys(self.system.module_parameters)
params = _common_keys(
tuple(array.module_parameters for array in self.system.arrays))
if {'A0', 'A1', 'C7'} <= params:
return self.sapm, 'sapm'
elif {'a_ref', 'I_L_ref', 'I_o_ref', 'R_sh_ref', 'R_s',
Expand All @@ -730,10 +735,11 @@ def infer_dc_model(self):
elif {'pdc0', 'gamma_pdc'} <= params:
return self.pvwatts_dc, 'pvwatts'
else:
raise ValueError('could not infer DC model from '
'system.module_parameters. Check '
'system.module_parameters or explicitly '
'set the model with the dc_model kwarg.')
raise ValueError(
'Could not infer DC model from the module_parameters '
'attributes of system.arrays. Check the module_parameters '
'attributes or explicitly set the model with the dc_model '
'keyword argument.')

def sapm(self):
dc = self.system.sapm(self.results.effective_irradiance,
Expand Down Expand Up @@ -782,7 +788,7 @@ def pvwatts_dc(self):
"""Calculate DC power using the PVWatts model.

Results are stored in ModelChain.results.dc. DC power is computed
from PVSystem.module_parameters['pdc0'] and then scaled by
from PVSystem.arrays[i].module_parameters['pdc0'] and then scaled by
PVSystem.modules_per_string and PVSystem.strings_per_inverter.

Returns
Expand Down Expand Up @@ -891,7 +897,9 @@ def aoi_model(self, model):
self._aoi_model = partial(model, self)

def infer_aoi_model(self):
params = _common_keys(self.system.module_parameters)
module_parameters = tuple(
array.module_parameters for array in self.system.arrays)
params = _common_keys(module_parameters)
if {'K', 'L', 'n'} <= params:
return self.physical_aoi_loss
elif {'B5', 'B4', 'B3', 'B2', 'B1', 'B0'} <= params:
Expand All @@ -902,8 +910,8 @@ def infer_aoi_model(self):
return self.martin_ruiz_aoi_loss
else:
raise ValueError('could not infer AOI model from '
'system.module_parameters. Check that the '
'module_parameters for all Arrays in '
'system.arrays[i].module_parameters. Check that '
'the module_parameters for all Arrays in '
'system.arrays contain parameters for '
'the physical, aoi, ashrae or martin_ruiz model; '
'explicitly set the model with the aoi_model '
Expand Down Expand Up @@ -966,7 +974,9 @@ def spectral_model(self, model):

def infer_spectral_model(self):
"""Infer spectral model from system attributes."""
params = _common_keys(self.system.module_parameters)
module_parameters = tuple(
array.module_parameters for array in self.system.arrays)
params = _common_keys(module_parameters)
if {'A4', 'A3', 'A2', 'A1', 'A0'} <= params:
return self.sapm_spectral_loss
elif ((('Technology' in params or
Expand All @@ -976,8 +986,8 @@ def infer_spectral_model(self):
return self.first_solar_spectral_loss
else:
raise ValueError('could not infer spectral model from '
'system.module_parameters. Check that the '
'module_parameters for all Arrays in '
'system.arrays[i].module_parameters. Check that '
'the module_parameters for all Arrays in '
'system.arrays contain valid '
'first_solar_spectral_coefficients, a valid '
'Material or Technology value, or set '
Expand Down Expand Up @@ -1028,20 +1038,24 @@ def temperature_model(self, model):
# check system.temperature_model_parameters for consistency
name_from_params = self.infer_temperature_model().__name__
if self._temperature_model.__name__ != name_from_params:
common_params = _common_keys(tuple(
array.temperature_model_parameters
for array in self.system.arrays))
raise ValueError(
f'Temperature model {self._temperature_model.__name__} is '
f'inconsistent with PVSystem temperature model '
f'parameters. All Arrays in system.arrays must have '
f'consistent parameters. Common temperature model '
f'parameters: '
f'{_common_keys(self.system.temperature_model_parameters)}'
f'parameters: {common_params}'
)
else:
self._temperature_model = partial(model, self)

def infer_temperature_model(self):
"""Infer temperature model from system attributes."""
params = _common_keys(self.system.temperature_model_parameters)
temperature_model_parameters = tuple(
array.temperature_model_parameters for array in self.system.arrays)
params = _common_keys(temperature_model_parameters)
# remove or statement in v0.9
if {'a', 'b', 'deltaT'} <= params or (
not params and self.system.racking_model is None
Expand Down Expand Up @@ -1195,7 +1209,7 @@ def _eff_irrad(module_parameters, total_irrad, spect_mod, aoi_mod):
self.results.spectral_modifier, self.results.aoi_modifier))
else:
self.results.effective_irradiance = _eff_irrad(
self.system.module_parameters,
self.system.arrays[0].module_parameters,
self.results.total_irrad,
self.results.spectral_modifier,
self.results.aoi_modifier
Expand Down Expand Up @@ -1629,7 +1643,7 @@ def _prepare_temperature_single_array(self, data, poa):
self.results.cell_temperature = self._get_cell_temperature(
data,
poa,
self.system.temperature_model_parameters
self.system.arrays[0].temperature_model_parameters
)
if self.results.cell_temperature is None:
self.temperature_model()
Expand Down
Loading