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.

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

Sg2 implementation #1013

Merged
merged 6 commits into from Apr 21, 2023
Merged

Sg2 implementation #1013

merged 6 commits into from Apr 21, 2023

Conversation

mjprilliman
Copy link
Collaborator

-Attempt to speed up SPA implementation by integrating equations from SG2 algorithm
-Paper: https://www.sciencedirect.com/science/article/pii/S0038092X12002800
-Github: https://github.com/gschwind/sg2/tree/main/src
-Replaces const static 3D arrays of values with single line approximations to speed up runtime
-Does not replace Greenwich mean sidereal time (v0) calculation as I haven't figured out the conversion (in lib_irradproc.cpp):
image

-ToDo: update test results (changes on the order of 1e-4)
-ToDo: remove unused commented code

@mjprilliman mjprilliman self-assigned this Apr 18, 2023
@mjprilliman mjprilliman added enhancement pv photovoltaic, pvsam, pvwatts labels Apr 18, 2023
@mjprilliman mjprilliman added this to the 2022.11.21 Patch 2 milestone Apr 18, 2023
Copy link
Collaborator

@sjanzou sjanzou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good....

I sent the script for the DC_AC_ratio tests to regenerate the json files
Tests that are currently failing:
image

@mjprilliman
Copy link
Collaborator Author

FYI @kandersolar from our previous discussions we are finding significant speed up from the SG2 modifications to SPA.

@kandersolar
Copy link
Member

Thanks for the heads up @mjprilliman!

Of course it would be a much bigger job than merely dropping in a new algorithm under the hood, but rather than replacing the SPA, would it be worthwhile to implement both SPA and SG2 and allow the user to choose which one is used? (it's easy for me to make suggestions, as someone that doesn't have to put in the work to make it happen 😄)

@mjprilliman
Copy link
Collaborator Author

Thanks for the heads up @mjprilliman!

Of course it would be a much bigger job than merely dropping in a new algorithm under the hood, but rather than replacing the SPA, would it be worthwhile to implement both SPA and SG2 and allow the user to choose which one is used? (it's easy for me to make suggestions, as someone that doesn't have to put in the work to make it happen 😄)

That may be something we consider in the future, right now we needed a solution that could go out a bit quicker. In reading SG2 it is essentially SPA modified in ~4 places from the original SPA algorithm so we'd have to weigh the workload versus potential optionality of allowing for both.

Copy link
Collaborator

@cpaulgilman cpaulgilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I ran some simulation run time tests for PVWatts, Detailed PV and Physical Trough in both SAM and SDKtool with results here:
simulation-runtime-results.zip

pvwattsv8 runs twice as fast with SG2 than SPA on my Windows 11 laptop with average run times of 374 ms for SPA and 174 ms for SG2.

@mjprilliman
Copy link
Collaborator Author

Looks good. I ran some simulation run time tests for PVWatts, Detailed PV and Physical Trough in both SAM and SDKtool with results here: simulation-runtime-results.zip

pvwattsv8 runs twice as fast with SG2 than SPA on my Windows 11 laptop with average run times of 374 ms for SPA and 174 ms for SG2.

Interesting. When @sjanzou ran comparisons for pvwattsv5 (to compare back to before original SPA was implemented) we were seeing ~3x speed increase. Hopefully 2x speed up still works for pvwattsv8 usage?

ssc2023.4.14_spa_sg2.zip

@cpaulgilman
Copy link
Collaborator

Looks good. I ran some simulation run time tests for PVWatts, Detailed PV and Physical Trough in both SAM and SDKtool with results here: simulation-runtime-results.zip
pvwattsv8 runs twice as fast with SG2 than SPA on my Windows 11 laptop with average run times of 374 ms for SPA and 174 ms for SG2.

Interesting. When @sjanzou ran comparisons for pvwattsv5 (to compare back to before original SPA was implemented) we were seeing ~3x speed increase. Hopefully 2x speed up still works for pvwattsv8 usage?

ssc2023.4.14_spa_sg2.zip

I just ran tests comparing pvwattsv5 in SSC 279 (SPA) and SSC 280 (SG2) and see similar results to Steve's: 297 ms vs 87 ms. SSC 242 (pre-SPA) is 55 ms. The PVWatts API team will probably want to do some tests on pvwattsv8 with SG2 to see how this affects the API run times -- I know they are working on some changes to the API to speed things up as well.

Copy link
Collaborator

@sjanzou sjanzou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and great that @cpaulgilman got similar speed up results and all ssc tests pass.

@mjprilliman mjprilliman merged commit 8536853 into patch Apr 21, 2023
4 checks passed
@mjprilliman mjprilliman deleted the sg2-implementation branch April 21, 2023 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement pv photovoltaic, pvsam, pvwatts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants