Conversation
|
Once this is accepted, next PR will replace calls to scipy's |
|
Put this in pvlib docs don't provide links to public function in either of these folders so there's no API change. |
echedey-ls
left a comment
echedey-ls
left a comment
There was a problem hiding this comment.
In general terms, I'd prefer to keep this function internal. Or at least give it some internal period just in case anything arises later. A function in the API is somewhat a contract with the user, and even thou I'm confident that it works pretty well, I'm not sure about the maintenance purely numerical API may bring later, or if users start assuming it follows numpy/scipy conventions... On the other hand, the more people that use it, the better to confirm that it works. I don't have a strong opinion.
Last big comment: what is the source of the equations? scipy's implementation? any source linked by them? I would add it for the sake of completeness. Sry if I missed it previously.
I don't mind its placement; thou I prefer pvlib.ivtools.utils, the nearest junk drawer to where it will be used.
Some minor comments below.
tests/test_tools.py
Outdated
| def test_lambertw_pvlib(): | ||
| test_exp = np.arange(-10., 300, step=10) | ||
| test_x = 10.**test_exp | ||
| # known solution from scipy.special.lambertw |
There was a problem hiding this comment.
Can you please add the exact version of scipy and date you generated it? For traceability. Else, we could generate the comparison on the fly, so if there ever is a change at scipy we can catch it soon.
Co-authored-by: Echedey Luis <80125792+echedey-ls@users.noreply.github.com>
It's direct application of two well-known methods (Halley's, Newton) to the Lambert W function. I can write a derivation (a few lines) in the function documentation, if that helps.
I agree, moved to ivtools.utils |
2 participants
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.Add a real-valued-only Lamberts W function. As tested, function is as precise as scipy's lambertw, and 4x faster due to not doing all arithmetic with complex values.