Add functions to fit and convert IAM models #1827
Conversation
The fitting doesn't require evenly spaced IAM measurements. |
|
Here's the reference, now published but it won't appear on OSTI or other sources for a few months. |
|
Ready for another round of reviews. @kandersolar I didn't make the residual function a user input, if that's important I can revisit. |
In the case of nonuniform samples, more weight being given to the regions of denser samples, I think. Not necessarily a problem, but something to consider documenting. |
| # data. | ||
|
|
||
| # Create and perturb IAM data. | ||
| aoi = np.linspace(0, 90, 100) |
There was a problem hiding this comment.
| aoi = np.linspace(0, 90, 100) | |
| aoi = np.linspace(0, 85, 18) |
This would more representative for measurements.
| # %% | ||
| # The weight function | ||
| # ------------------- | ||
| # :py:func:`pvlib.iam.fit` uses a weight function when computing residuals |
There was a problem hiding this comment.
I suggest you say a bit more about the shape and purpose of these functions. I see there is more in the other example, but I think it is more relevant or important here.
| \\sum_{\\theta=0}^{90} weight \\left(\\theta \\right) \\times | ||
| \\| source \\left(\\theta \\right) - target \\left(\\theta \\right) \\| | ||
|
|
||
| The sum is over :math:`\\theta = 0, 1, 2, ..., 90`. |
There was a problem hiding this comment.
Not quite true; the code actually uses np.linspace(0, 90, 100)
There was a problem hiding this comment.
Changed to code to linspace(0, 90, 90)
Ready to merge from my point of view.
There was a problem hiding this comment.
Pedantic nitpick: linspace(0, 90, 90) produces [0., 1.0112, 2.0225, ... , 87.9775, 88.9888, 90.]. To produce theta = 0, 1, 2, ..., 90 as the docstring states, I think we need linspace(0, 90, 91).
There was a problem hiding this comment.
Not a nit, rather, that's the intent, because it feels natural to step by 1.0000000000000...
There was a problem hiding this comment.
I agree it feels natural to step by unity. I was trying to point out that the current code (np.linspace(0, 90, 90)) steps by 90/89 linspace(0, 90, 91).
|
@kandersolar codecov error. |
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.