Add optional initial guess to fit_desoto #2291
Conversation
| @@ -166,6 +166,9 @@ def fit_desoto(v_mp, i_mp, v_oc, i_sc, alpha_sc, beta_voc, cells_in_series, | |||
| Reference temperature condition [C] | |||
| irrad_ref: float, default 1000 | |||
| Reference irradiance condition [W/m2] | |||
| init_guess: dict, optional | |||
| Initial values for optimization, must have keys `'Rsh_0'`, `'a_0'`, | |||
| `'IL_0'`, `'Io_0'`, `'Rs_0'`. | |||
There was a problem hiding this comment.
Could it be useful to accept a subset of these as input? (And use the internal values for the rest?)
There was a problem hiding this comment.
Good suggestion. Convergence of this method appears to be somewhat sensitive to the initial guess and the built-in may not always work.
| @@ -120,107 +120,121 @@ def fit_cec_sam(celltype, v_mp, i_mp, v_oc, i_sc, alpha_sc, beta_voc, | |||
|
|
|||
| def fit_desoto(v_mp, i_mp, v_oc, i_sc, alpha_sc, beta_voc, cells_in_series, | |||
| EgRef=1.121, dEgdT=-0.0002677, temp_ref=25, irrad_ref=1000, | |||
| root_kwargs={}): | |||
| init_guess={}, root_kwargs={}): | |||
There was a problem hiding this comment.
Best practice is to avoid mutable types for default parameter values, but this function already has another one (root_kwargs) so I won't object to it.
There was a problem hiding this comment.
Understood. I'm happy to replace that default if we concoct a clean way to pass that input. Positions in a tuple feel clumsy; if init_guess is not None: if isinstance(init_guess, dict) is an option, I suppose.
Co-authored-by: Kevin Anderson <[email protected]>
|
Thanks @cwhanse! |
docs/sphinx/source/referencefor API changes.docs/sphinx/source/whatsnewfor all changes. Includes link to the GitHub Issue with:issue:`num`or this Pull Request with:pull:`num`. Includes contributor name and/or GitHub username (link with:ghuser:`user`).remote-data) and Milestone are assigned to the Pull Request and linked Issue.Also corrects the references in the docstring of fit_desoto