Fix to WVM methodology to track changes in MATLAB version. #1213
Conversation
…to be done here to get the wavelet levels to match the MATLAB output. The only place python and MATLAB don't agree now is at the edges, where there are different assumptions about how the moving average should be computed for the beginning/end of the time series. In principle, this could actually matter for time series that aren't "much" longer than 4096 seconds.
There was a problem hiding this comment.
@jranalli looks good for the bug fix. We just set up the Matlab code for the WVM on github, and the initial load included the corrections from @mlave. I'd recommending pointing reference [2] to https://github.com/sandialabs/wvm
pvlib/scaling.py
Outdated
| [2] Wavelet Variability Model - Matlab Code: | ||
| https://pvpmc.sandia.gov/applications/wavelet-variability-model/ |
There was a problem hiding this comment.
| [2] Wavelet Variability Model - Matlab Code: | |
| https://pvpmc.sandia.gov/applications/wavelet-variability-model/ | |
| .. [2] Wavelet Variability Model - Matlab Code: | |
| https://pvpmc.sandia.gov/applications/wavelet-variability-model/ |
Could you edit the format for [1] as well? Two leading dots before [1], and align the following lines with the "]" character.
There was a problem hiding this comment.
Since that same reference is in the other functions, I updated all of those as well.
Co-authored-by: Cliff Hansen <[email protected]>
|
I think what I did incorporated your suggestion about language in the documentation. Sorry if not, I'm not all that great with github! |
Looks great. @kanderso-nrel @wholmgren @mikofski @mlave will merge tomorrow unless you indicate otherwise. |
|
@jranalli I made some minor tweaks to the documentation. Will merge when the tests complete. |
docs/sphinx/source/api.rstfor 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`).WVM wavelet computation was discarding the highest frequency scale wavelet coefficients. Partial discussion of the fix took place via email with the original WVM Matlab developer/maintainer and @cwhanse. There are some methodological quirks with retaining this wavelet scale (most of the coefficients represent a top hat wavelet, the omitted term is a Haar wavelet), but retaining it is necessary in order to properly use the summative inverse transform that the WVM is using. The impact on the smoothed output from the WVM code is essentially negligible, so it was agreed that it makes sense to implement this change to have more consistency in the back end. Test conditions for python port were updated to match the proposed fix in the Matlab version.
In the process, it seems that there was a time shift bug was in the moving average calculation, where the approximation and detail coefficients were shifted by one time step. I have corrected that, so both Matlab and python produce the same wavelets. I believe the only difference between the two at present is that the Matlab and python codes use different conventions for how to treat moving averages for the segments at the beginning/end of a time series. Given that WVM computes averages up to 4096 seconds, there is a possibility that this would result in differences results between the versions near the edges. There's no easy way to fix that without writing a custom moving average computation.