Repository navigation
Add support for diffuse irradiance components and component-specific IAM to ModelChain #2846
Description
I am currently at the final stage of my GSoC (#2750). After expanding support for diffuse components in transposition functions and adding support for diffuse component-specific IAM in the Array and PVSystem classes, the remaining step is to integrate this functionality into ModelChain.
Here are some points I think are worth discussing:
Default behavior
The existing behavior applied fd to all diffuse irradiance. fd was essentially always 1, meaning no optical losses were accounted for for diffuse irradiance.
What should the new default behavior be?
marion_diffuse is a good candidate for a default diffuse IAM model, since it is the only model that supports all three diffuse components: sky diffuse, horizon, and ground. (martin_ruiz_diffuse and schlick_diffuse support sky ground only).
As for marion_diffuse's model parameter (i.e. the direct IAM model used by the Marion integration method), I'm considering doing something like the existing infer_aoi_model which selects a model based on the available parameters when the user does not explicitly choose one.
FD
Do we want to keep FD around as an option?
Handling missing IAM for existing components
Let's say the user chooses the perez transposition model, and therefore diffuse irradiance includes the horizon component, and then chooses the martin_ruiz_diffuse IAM model, which does not provide an IAM for the horizon component. How should this be handled?
Options:
- Throw an error prompting the user to select a compatible combination of transposition and IAM models
- Apply the sky diffuse IAM to the horizon component
- Apply no optical losses to the horizon component
- Something else?
Activity
cwhanse commentedon Aug 19, 2026
My thoughts: keep
fdas a module parameter, and add a new kwarg toModelChainsayaoi_diffuse_modeldefault None. Ifaoi_diffuse_modelis present, then execute a new stepaoi_diffuse_modelinModelChain.run_modelmethods, which populatesModelChainResult.aoi_diffuse_modifier. Thenmodelchain.effective_irradiance_modelusesaoi_diffuse_modiferinside this little function:pvlib-python/pvlib/modelchain.py
Line 1064 in f025155
I'd default
aoi_diffuse_model = Noneto keep behavior consistent with prior releases.If a user provides
aoi_diffuse_model, the burden of choosing a matching transposition model should be on them. We could add some kind of logic that warns about incompatible combinations. To my thinking this is better than trying to make choices for a user.Where the
aoi_diffuse_modeldoesn't cover a component (e.g., horizon) we could supplement the output of the diffuse AOI model with 1s for the missing components so that all three components are covered in the output. That keeps the downstream code simpler.Final thought: this integration is a bit complicated so I'm OK if it we decide it's out of scope.
cbcrespo commentedon Aug 19, 2026
Personally, I don't agree with setting
aoi_diffuse_modelto None by default. I'd say most users likely won't personalise every parameter, and if the functionality exists, I think the default behaviour should be to make use of it to get more accurate results.To be clear, are you advocating for raising an error, a warning, or something else?
I would like to try :) even if code review and adjustments spill past my GSoC project timeline, I am willing to dedicate some more time to those adjustments. I already got started on the implementation, and I think I can have a PR ready by Friday (GSoC deadline is Monday), even if it isn't perfect yet.
cwhanse commentedon Aug 19, 2026
A warning, if we also choose to populate the missing IAMs with 1. Which is kind of the default anyway, if a diffuse AOI model doesn't address a component such as horizon irradiance, it shouldn't modify that component.
ramaroesilva commentedon Aug 19, 2026
thanks for the input @cwhanse. follow-up from me:
Just curious, what's the point of having a parameter which is always
1(at least within pvlib's built-in module database)? Are there module descriptors outside pvlib's database - likely not public - that have non-1values?While I understand the idea, wouldn't it be better to have a more robust configuration as default? Having
marion+spamfor cases with a more well-described PV system andmarion+martin_ruizfor PVWatts-like cases seems to me as a solid default. This could be even better ifhaydavieswas replaced withperez- proposed by @AdamRJensen in #1861 - since it includes horizon.Seems reasonable to me. Better to ignore some diffuse losses than all of them, and if
marionbecomes the default, it will cover all components.Being @cbcrespo, the odds are good for us :-)
cwhanse commentedon Aug 19, 2026
None that I know of. That equation derives from the SAPM, which (for a time) was being used for CPV, so perhaps that's where the
fdparameter originates.I object to defaulting to a particular diffuse AOI model for two reasons: we'd have to choose a default model, and out of respect for current users.
I don't think choosing a default AOI / diffuse AOI model is pvlib's job. We make users explicitly select an
aoi_modelmodel (or None for no model) when a ModelChain is instantiated. That's better than silently choosing for them. Some would argue that pvlib shouldn't have chosen default clear-sky or transposition models, either. As @AdamRJensen points out, a default model is unlikely to be the best choice for most applications. We've attempted to avoid positioning pvlib as a referee of "best" models although some judgment is inevitable (otherwise pvlib would have dozens if not hundreds of diffuse irradiance models of minor interest, for example).If we set
aoi_diffuse_modelto one of the specific models, then existing code will silently run and the results will be slightly different than any past archived results. That's not good. If we require a user to choose anaoi_diffuse_model, existing code won't run, so this is a breaking change requiring deprecation.For these reasons, I prefer defaulting to
aoi_diffuse_model = None.ramaroesilva commentedon Aug 20, 2026
Thank you for further explaining your point of view, I know it takes time and effort to frame and explain one's own reasoning to others.
All good for me!
echedey-ls commentedon Aug 20, 2026
Just a heads up: the new parameter should probably be called
iam_diffuse_modelrather thanaoi_diffuse_model, as per #2748. I agree with everything else in this conversation. Thank you @cbcrespo for continuing development after GSoC deadline!