PV models speed up #1212

dguittet · 2024-09-30T19:06:58Z

Speed up pvsamv1, pvwattsv7 and pvwattsv8 by speeding up irradproc and sssky_diffuse_table.

irradproc:

for intra-annual solarpos_spa calculation speed up, introduce a hash table that tracks the intermediate outputs of calculate_spa that reduces the number of times that calculate_spa is called per timestep
for lifetime calculation, inter-annual speed up is done by storing the results of solarpos_spa in a vector so that the values are not recalculated each year

sssky_diffuse_table:

change the keys of the hash table to numeric key instead of string since sprintf takes a long time
reduce the number of steps/samples required to compute the self-shading loss in sssky_diffuse_table::compute

pvwattsv8 and pvwattsv7:

move irrad proc outside the time loop

pvsamv1:

move irrad proc outside the time loop
change the constructor used for irradproc

tests:

pvsamv1 test values are changed (shows how the actual value changed)
pvwattsv8 test tolerances are changed (shows how much the relative error increased in order to use the old expected values). A potential additional change in this PR is to change the expected values to the new values

Speed changes

Before is the original code, "Spa Hash" is what is implemented in this PR

mjprilliman

This looks great! I want to double check seasonal tilt behavior, so I may need to do a bit more debugging.

shared/lib_irradproc.cpp

test/ssc_test/cmod_pvsamv1_test.cpp

shared/lib_irradproc.cpp

mjprilliman

Looks good to me. Just one merge conflict from recent merges to address

janinefreeman · 2024-10-02T16:45:04Z

This looks good to me! I think it makes sense to update the pvwattsv8 expected test values to the new values as well. I like scaling the error tolerance by annual energy, but I think I'd like to make the error tolerance itself smaller if we're keeping that approach.

Requesting an additional review from @sjanzou , particularly on the hash table, just because this is a big change for pvwatts and pvsam.

Super excited for these results @dguittet !!! Nice job!!!

dguittet · 2024-10-02T21:58:21Z

Need tests for Alaska and South Africa

sjanzou

Looks good overall, just a few comments and questions on some of the implementation values and sensitivities.

Thanks for tackling this!

sjanzou · 2024-10-04T09:22:14Z

test/ssc_test/cmod_pvwattsv5_test.cpp

@@ -91,7 +91,7 @@ TEST_F(CMPvwattsV5Integration_cmod_pvwattsv5, DifferentTechnologyInputs)
 {
 //	std::vector<double> annual_energy_expected = { 6909.79, 7123.32, 7336.478, 6909.79, 6804.376, 8711.946, 8727.704, 9690.735 };
 	// single axis tracking reduction due to pull request 280
-	std::vector<double> annual_energy_expected = { 6908.13, 7121.63, 7334.83, 6908.13, 6802.72, 8633.37, 8721.21, 9687.06 };
+	std::vector<double> annual_energy_expected = { 6908.13, 7121.63, 7334.83, 6908.13, 6802.72, 8632.33, 8721.21, 9687.06 };


Seems like the general trend is for lower annual energy in all the tests and the default configuration results.

sjanzou · 2024-10-04T09:25:43Z

shared/lib_irradproc.cpp

+        ascension_and_declination[0], ascension_and_declination[1]};
+
+    if (use_table)
+        spa_table[spa_key_inputs] = spa_outputs;


Have you run with use_table = false to verify previous results?

sjanzou · 2024-10-04T09:28:58Z

shared/lib_irradproc.cpp

+}
+
+bool irrad::getStoredSolarposOutputs() {
+    if (!solarpos_outputs_for_lifetime.size()) return false;


Is it clearer for maintenance to use if (solarpos_outputs_for_lifetime.size() == 0) return false;

sjanzou · 2024-10-04T09:32:58Z

shared/lib_irradproc.h

+    spa_table_key(double j, double dt, double p, double t, double a, double d):
+        jd(j), delta_t(dt), ascension(a), declination(d)
+    {
+        int pressure_bucket = 10;


Do you have a sensitivity plot based on pressure and temp bucket size?

sjanzou · 2024-10-04T09:44:05Z

shared/lib_irradproc.h

+    // Compute individual hash values for first, second, etc and combine them using XOR and bit shifting:
+    return 
+    ((((
+            ((((hash<double>()(k.jd)


Have you tried hash_combine in C++17 for more robustness (i.e. less collisions)?

sjanzou · 2024-10-04T09:47:43Z

shared/lib_pvshade.cpp

-    sprintf(buf, "%.3f", surface_tilt);
-    if (derates_table.find(buf) != derates_table.end())
-        return derates_table[buf];
+    int surface_tilt_key = int(surface_tilt * derates_table_digits_multiplier);


This seems like it would be faster - nice approach!

sjanzou · 2024-10-04T09:48:45Z

shared/lib_pvshade.cpp

    return compute(surface_tilt);
 }

 double sssky_diffuse_table::compute(double surface_tilt) {
    if (gcr == 0)
        throw std::runtime_error("sssky_diffuse_table::compute error: gcr required in initialization");
    // sky diffuse reduction
-    double step = 1.0 / 1000.0;
+	const size_t n_steps = 250;


Do you have a sensitivity analysis on some different n_steps values?

dguittet and others added 15 commits May 6, 2024 09:15

updateCapacityForLifetime increase-able capacity

9036f53

update test value

09460d8

Merge branch 'patch' into batt_thermal

d5b6d83

Merge branch 'batt_thermal' into pvwatts_speed

b4ca663

Merge branch 'patch' into pvwatts_speed

8009fec

use relative tolerance for pv tests & speed up sssky_diffuse_table

0f174ca

fix import of pow

2ae4e2a

add solarpos_spa hash table

98ba011

fix test

d84dbdd

fix windows build

4229512

Update save_as_JSON_test.cpp

409a88e

add lifetime table to irradproc

3692ef3

update for pvsamv1

04a9bd0

Merge branch 'develop' into pvwatts_speed

0fbfc8c

fix pvsamv1 test results

221afbc

dguittet requested review from mjprilliman and janinefreeman September 30, 2024 21:34

mjprilliman reviewed Oct 1, 2024

View reviewed changes

shared/lib_irradproc.cpp Show resolved Hide resolved

test/ssc_test/cmod_pvsamv1_test.cpp Outdated Show resolved Hide resolved

mjprilliman reviewed Oct 1, 2024

View reviewed changes

shared/lib_irradproc.cpp Show resolved Hide resolved

revert test results

0cf843c

mjprilliman approved these changes Oct 2, 2024

View reviewed changes

dguittet added 2 commits October 2, 2024 08:52

Merge branch 'develop' into pvwatts_speed

222f4b2

modify test results for cmod_hybrid_test

2b20808

janinefreeman approved these changes Oct 2, 2024

View reviewed changes

janinefreeman requested a review from sjanzou October 2, 2024 16:45

dguittet added 3 commits October 2, 2024 14:39

update test results for pvwattsv8

7b3a2be

update PVwattsv8 tests w/ DCACRatio*

400d62c

update results for cmod_pvwattsv7 and v5

1b107b2

update sam_pv_test result

4d5d516

sjanzou added a commit to NREL/SAM that referenced this pull request Oct 4, 2024

api updates and default changes from NREL/ssc#1212

26018f5

sjanzou mentioned this pull request Oct 4, 2024

api updates and default changes from https://github.com/NREL/ssc/pull/1212 NREL/SAM#1863

Open

sjanzou reviewed Oct 4, 2024

View reviewed changes

:merge branch 'develop' into pvwatts_speed

3d7dc05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PV models speed up #1212

PV models speed up #1212

dguittet commented Sep 30, 2024 •

edited

Loading

mjprilliman left a comment

mjprilliman left a comment

janinefreeman commented Oct 2, 2024

dguittet commented Oct 2, 2024

sjanzou left a comment

sjanzou Oct 4, 2024

sjanzou Oct 4, 2024

sjanzou Oct 4, 2024

sjanzou Oct 4, 2024

sjanzou Oct 4, 2024

sjanzou Oct 4, 2024

sjanzou Oct 4, 2024

PV models speed up #1212

Are you sure you want to change the base?

PV models speed up #1212

Conversation

dguittet commented Sep 30, 2024 • edited Loading

Speed changes

mjprilliman left a comment

Choose a reason for hiding this comment

mjprilliman left a comment

Choose a reason for hiding this comment

janinefreeman commented Oct 2, 2024

dguittet commented Oct 2, 2024

sjanzou left a comment

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

sjanzou Oct 4, 2024

Choose a reason for hiding this comment

dguittet commented Sep 30, 2024 •

edited

Loading