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

ModelChain: unexpected errors when building with module from CECMod #621

Closed
jgaffiot opened this issue Nov 20, 2018 · 4 comments · Fixed by #703
Closed

ModelChain: unexpected errors when building with module from CECMod #621

jgaffiot opened this issue Nov 20, 2018 · 4 comments · Fixed by #703

Comments

@jgaffiot
Copy link

Not sure this issue is a bug or a misuse from my side, but here is my problem.
When trying to build a ModelChain with module parameters found in the CEC database, several errors arise:

  • if no AOI model is provided, I get ValueError: could not infer AOI model from system.module_parameters
  • then using the physical AOI model (chosen because I found in the code that this model has default parameters), when trying to run, I get KeyError: 'precipitable_water'. It appears that the default spectral model expects 'precipitable_water' in weather data, and I could not find information on such a parameter in the documentation.
  • then the only spectral model working is no_loss.

To Reproduce

from pvlib.location import Location
from pvlib.pvsystem import retrieve_sam, PVSystem
from pvlib.modelchain import ModelChain
inv_par = retrieve_sam("CECInverter")["ABB__MICRO_0_25_I_OUTD_US_208_208V__CEC_2014_"]
mod_par = retrieve_sam("CECMod")["SunPower_SPR_E20_435_COM"]
loc = Location(0.,0.)
pv = PVSystem(inverter_parameters=inv_par, module_parameters=mod_par)
mc = ModelChain(pv, loc, orientation_strategy="None")

then

mc = ModelChain(pv, loc, orientation_strategy="None", aoi_model="physical")
mc.run_model()

Expected behavior
Or a running ModelChain or clear errors indicating the bad parametrization, and potentially where to find information.

Versions:

  • pvlib.__version__: 0.6.0
  • pandas.__version__: 0.23.4
  • python: 3.6
@cwhanse
Copy link
Member

cwhanse commented Nov 20, 2018

Reproduced the behavior after fixing #620. You are correct, without module-specific parameters for one of the spectral modifier models, the only choice is spectral_model="no_loss". Adding this keyword argument to

mc = ModelChain(pv, loc, orientation_strategy="None", aoi_model="physical")

gets passed the model chain definition, if that advances your work.

We haven't specified a default AOI model nor a default spectral model. The 'default' AOI model parameters are considered representative for untreated flat-glass modules. We could discuss making physicaliam() the default, since that is what is done by the CEC and SAM.

There are no default parameters for the spectral models, and the CEC database does not include values. The FirstSolar model requires precipitable water as input, which can be calculated from RH and air temperature using pvlib.atmosphere.gueymard94_pw but ModelChain doesn't do this for you, currently (PR welcome :).

@wholmgren
Copy link
Member

I think ModelChain is behaving as expected. Perhaps it can be documented better or the error messages can be more helpful (like suggesting users specify 'no_loss'). I deliberately chose to make this fail rather than default to 'no_loss'. As they say in import this: "Explicit is better than implicit." and "In the face of ambiguity, refuse the temptation to guess."

@jgaffiot
Copy link
Author

jgaffiot commented Nov 20, 2018

The behavior of the ModelChain is not consistent: with the Sandia module database, it works out of the box, for the CEC module database, you get 2 crashes with little information.
It would make sense to define sensible default values with a warning message ("this model has been chosen by default, see pvlib-python.readthedocs.io/CECdefault-or-whatever for details") in the case of the CEC modeling. At least, the error message should explain that CEC models are incomplete and that the user needs to choose an AOI model, and in practice little choice but to choose the no loss spectral model.
I will try to propose something in this direction, but tomorrow :).

@Hendrick42
Copy link

Who is still having the CEC issue with the precipitable_water issue, this works for me:

weather["precipitable_water"] = pvlib.atmosphere.gueymard94_pw(weather["temp_air"], weather["relative_humidity"])  # needed for aoi_model="physical" if using CECModueles
modelchain.run_model(weather)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants