Skip to content

ModelChain claims it supports module_parameters from CEC, but only uses sapm internally #177

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

Closed
jforbess opened this issue May 21, 2016 · 3 comments
Milestone

Comments

@jforbess
Copy link
Contributor

Am I confused, or should there be a check as to whether the module params are CEC or Sandia, because you have to use different models depending on which set of parameters you're using?

@wholmgren
Copy link
Member

I certainly never meant to claim that ModelChain supported CEC modules or singlediode! I'd readily accept a PR that fixes documentation or makes it more clear. Have you seen PR #151 that refactors ModelChain and implements SAPM and SingleDiode classes? It needs to be rebased on the latest changes, but it's mostly complete. I want to get 0.3.3 out before merging that and switching development to the 0.4 series.

@jforbess
Copy link
Contributor Author

Yeah, the parameter definition is: "Module parameters as defined by the
SAPM, CEC, or other."

Ah, yes, PR #151. I thought I had seen it go through, but I only checked
Issues, not PRs. I will have to look at it when I have more time. I am
motivated to get singlediode in there, because that's what I use, but I had
to stop with the ModelChain work today when I hit this issue, because I
have a deadline, as usual.

On Sat, May 21, 2016 at 7:52 PM, Will Holmgren [email protected]
wrote:

I certainly never meant to claim that ModelChain supported CEC modules or
singlediode! I'd readily accept a PR that fixes documentation or makes it
more clear. Have you seen PR #151
#151 that refactors
ModelChain and implements SAPM and SingleDiode classes? It needs to be
rebased on the latest changes, but it's mostly complete. I want to get
0.3.3 out before merging that and switching development to the 0.4 series.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#177 (comment)

@wholmgren wholmgren added this to the 0.3.3 milestone May 22, 2016
@wholmgren
Copy link
Member

Thanks for pointing that out. I made quick update to the documentation to clarify the current status.

I also rebased the code in #151, pushed it to a new branch, and opened a new PR from that branch.

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

No branches or pull requests

2 participants