Skip to content
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

[BUG]: Infer spectral models correctly #2253

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open
Changes from 2 commits
Commits
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
73 changes: 56 additions & 17 deletions pvlib/modelchain.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,15 @@ class ModelChain:
'interp' and 'no_loss'. The ModelChain instance will be passed as the
first argument to a user-defined function.

spectral_model : str, or function, optional
spectral_model : str or function, optional
If not specified, 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'.
are common to all of system.arrays[i].module_parameters. Valid strings
echedey-ls marked this conversation as resolved.
Show resolved Hide resolved
are:

- ``'sapm'``
- ``'first_solar'``
- ``'no_loss'``

The ModelChain instance will be passed as the first argument to
a user-defined function.

Expand Down Expand Up @@ -855,9 +860,7 @@ def spectral_model(self):

@spectral_model.setter
def spectral_model(self, model):
if model is None:
self._spectral_model = self.infer_spectral_model()
elif isinstance(model, str):
if isinstance(model, str):
model = model.lower()
if model == 'first_solar':
self._spectral_model = self.first_solar_spectral_loss
Expand All @@ -867,6 +870,9 @@ def spectral_model(self, model):
self._spectral_model = self.no_spectral_loss
else:
raise ValueError(model + ' is not a valid spectral loss model')
elif model is None:
# None is a valid value, model inference will be done later
self._spectral_model = None
else:
self._spectral_model = partial(model, self)

Expand All @@ -877,19 +883,46 @@ def infer_spectral_model(self):
params = _common_keys(module_parameters)
if {'A4', 'A3', 'A2', 'A1', 'A0'} <= params:
return self.sapm_spectral_loss
elif ((('Technology' in params or
'Material' in params) and
(self.system._infer_cell_type() is not None)) or
'first_solar_spectral_coefficients' in params):
elif "first_solar_spectral_coefficients" in params:
echedey-ls marked this conversation as resolved.
Show resolved Hide resolved
# user explicitly sets spectral coefficients
return self.first_solar_spectral_loss
elif (
# cell type is known or can be inferred
("Technology" in params or "Material" in params)
and (self.system._infer_cell_type() is not None)
):
# This suggests models that provide default parameters per cell
# type can be used. However, some of them depend on other weather
echedey-ls marked this conversation as resolved.
Show resolved Hide resolved
# parameters, so we need to check if they are available.
# At this point, weather may not be available, so let's take that
# into account.
if self.results is not None and self.results.weather is not None:
echedey-ls marked this conversation as resolved.
Show resolved Hide resolved
# weather is available
if "precipitable_water" in self.results.weather:
return self.first_solar_spectral_loss
else:
return self.no_spectral_loss
else:
# weather is not available, so it's unknown what model to use.
echedey-ls marked this conversation as resolved.
Show resolved Hide resolved
# Warn user about this and default to no_spectral_loss.
warnings.warn(
"Weather data is not available to infer spectral model. "
"Defaulting to 'no_loss'. Explicitly set the "
"spectral model with the 'spectral_model' kwarg to "
"suppress this warning."
)
return self.no_spectral_loss
else:
raise ValueError('could not infer spectral model from '
'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 '
'spectral_model="no_loss".')
raise ValueError(
"could not infer spectral model from "
"system.arrays[i].module_parameters. Check that "
"the module_parameters for all Arrays in "
"system.arrays contain valid "
"valid spectral coefficients, a valid "
"Material or Technology value, or a valid model "
"string; explicitly set the model with the "
"'spectral_model' kwarg."
)

def first_solar_spectral_loss(self):
self.results.spectral_modifier = self.system.first_solar_spectral_loss(
Expand Down Expand Up @@ -1678,7 +1711,13 @@ def run_model(self, weather):
weather = _to_tuple(weather)
self.prepare_inputs(weather)
self.aoi_model()

# spectral model is optional, and weather may impact model inference
# check if spectral model inference is requested by means of "None"
if self._spectral_model is None:
self._spectral_model = self.infer_spectral_model()
self.spectral_model()

self.effective_irradiance_model()

self._run_from_effective_irrad(weather)
Expand Down
Loading