Skip to content

Reduce required inputs for ModelChain.run_model_from_effective_irradiance #1411

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

Open
cwhanse opened this issue Feb 24, 2022 · 2 comments
Open

Comments

@cwhanse
Copy link
Member

cwhanse commented Feb 24, 2022

ModelChain.run_model_from_effective_irradiance requires that the ModelChain instance has attributes which the method doesn't use, e.g., ModelChain.aoi_model. It would be nice if the method runs without requiring these attributes.

"It's a small inelegance that specifying aoi_model is necessary for a ModelChain that only uses run_model_from_effective_irradiance. I don't see a way around it, so I don't think there's anything to be done in this PR, just thinking out loud about the ModelChain design."

Originally posted by @kanderso-nrel in #1394 (comment)

@cwhanse cwhanse changed the title It's a small inelegance that specifying aoi_model is necessary for a ModelChain that only uses run_model_from_effective_irradiance. I don't see a way around it, so I don't think there's anything to be done in this PR, just thinking out loud about the ModelChain design. Reduce required inputs for ModelChain.run_model_from_effective_irradiance Feb 24, 2022
@wholmgren
Copy link
Member

It would be nice if the method runs without requiring these attributes.

Isn't the problem that the method must be specified when the ModelChain object is instantiated? If so, a better design might be different ModelChain classes such as ModelChain3Component, and ModelChainEffectiveIrradiance. Then each would have its own run_model rather than run_model_.*. Of course that's a pretty substantial API change and may or may not be worth it. Setting different defaults and raising errors at runtime if needed would be a relatively small change, but is less consistent with one of the original goals of ModelChain (if you can build it, you can run it).

@cwhanse
Copy link
Member Author

cwhanse commented Feb 24, 2022

Isn't the problem that the method must be specified when the ModelChain object is instantiated?

Yes, and please feel at liberty to edit the issue title and description.

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

No branches or pull requests

2 participants