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.

Sign up for GitHub

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

Jump to bottom

DOC: remove very long voc estimate equations from singlediode #518

Closed
mikofski opened this issue Aug 3, 2018 · 1 comment · Fixed by #525
Closed

DOC: remove very long voc estimate equations from singlediode #518

mikofski opened this issue Aug 3, 2018 · 1 comment · Fixed by #525
Labels
documentation
Milestone
0.6.0

Comments

@mikofski
Copy link
Member

mikofski commented Aug 3, 2018 •
edited
Loading

Problem
PR #409 added a very long and redundant set of equations in singlediode that are inappropriate there

Solution
remove it from the docstring in singlediode and move into an appendix of the sphinx source

Alternatives

  • just delete it and state that estimated Voc has been shown to be in the 4th quadrant.
  • move it to singlediode.estimate_voc
  • abbreviate it so it's not so long

Additional context
vocest-proof
@mikofski mikofski changed the title remove very long voc estimate equations from singlediode BUG: remove very long voc estimate equations from singlediode Aug 3, 2018
@mikofski mikofski changed the title BUG: remove very long voc estimate equations from singlediode DOC: remove very long voc estimate equations from singlediode Aug 3, 2018
@cwhanse
Copy link
Member

cwhanse commented Aug 3, 2018

Concur with recommended solution, I don't like the alternatives (move it elsewhere in the code, or delete it). This kind of stuff belongs in a manual.

@wholmgren wholmgren added this to the 0.6.0 milestone Aug 7, 2018
mikofski added a commit to mikofski/pvlib-python that referenced this issue Aug 8, 2018
@mikofski
DOC: move proof that estimated Voc is in q4
1a42153 
* closes pvlib#518
* add singlediode.rst and add to index
@mikofski mikofski mentioned this issue Aug 8, 2018
Merged
8 tasks
wholmgren pushed a commit that referenced this issue Aug 10, 2018
@mikofski @wholmgren
MAINT: DOC: rename singlediode module, and clean up docstring for sin…
dc6bdbf 
…glediode function (#525)

* MAINT: rename singlediode_methods.py to just singlediode.py

* closes #515
* import pvlib in pvsystem and use pvlib.singlediode to differentiate
from existing pvsystem.singldiode() method already there!
* rename test_singlediode_methods.py to test_singlediode.py
* update documentation and comments

* DOC: move proof that estimated Voc is in q4

* closes #518
* add singlediode.rst and add to index

* DOC: add DOI to Jain and Kapoor

* proofed the docs and they are ok

* DOC: MAINT: improve wording in singlediode.rst

* add to what's new documentation section that there is now some detail
on pvlib-python solutions to single diode equation
* change single diode _model_ to single diode _equation_ everywhere to
distinguish between the equation I = IL - I0*(exp(Vd/nNsVt)-1) - Vd/Rsh
and implementation like PVSyst and DeSoto that derive coefficients
* add reference to Cliff's Sandia Report on Lambert W-function solution
of single diode equation
* remove fixme in pvsystem

* DOC: MAINT: fix citation for Sandia Report, remove extra parentheses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
documentation
Projects
None yet
Milestone
0.6.0
3 participants
@mikofski @wholmgren @cwhanse