arm64 failing ssc tests

[sjanzou]

Four failing ssc tests on arm64 architecture (all 725 run fine on amd64 Windows, Linux and Mac)

1. lib_battery_test.runTestCycleAt3C

2. csp_common.TesSubcomponentCmod.Default

3. CMPvsamv1BatteryIntegration_cmod_pvsamv1.ResidentialDCBatteryModelPriceSignalDispatch

4. CMBattwatts_cmod_battwatts.NoPV

Detailed value difference for each test below

[0;32m[ RUN ] �[mlib_battery_test.runTestCycleAt3C
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_battery_capacity_test.h:45: Failure
The difference between tested_state.q0 and expected_state.q0 is 0.025430198576842145, which exceeds tol, where
tested_state.q0 evaluates to 47.115430198576846,
expected_state.q0 evaluates to 47.090000000000003, and
tol evaluates to 0.01.
runTest: 3
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_battery_test.h:142: Failure
The difference between tested_state.V and expected_state.batt_voltage is 0.02420895181734295, which exceeds 0.01, where
tested_state.V evaluates to 467.11420895181732,
expected_state.batt_voltage evaluates to 467.08999999999997, and
0.01 evaluates to 0.01.
runTest: 3
�[0;31m[ FAILED ] �[mlib_battery_test.runTestCycleAt3C (373 ms)
�[0;32m[----------] �[m1 test from lib_battery_test (373 ms total)

�

[0;32m[----------] �[m1 test from csp_common.TesSubcomponentCmod
�[0;32m[ RUN ] �[mcsp_common.TesSubcomponentCmod.Default
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_csp_tes_test.cpp:411: Failure
The difference between csp_subcomponent.GetOutputVector("T_sink_in")[idx] and T_sink_in_expected[idx] is 0.11852206011218414, which exceeds 0.1, where
csp_subcomponent.GetOutputVector("T_sink_in")[idx] evaluates to 383.69852206011217,
T_sink_in_expected[idx] evaluates to 383.57999999999998, and
0.1 evaluates to 0.10000000000000001.
at index: 18
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_csp_tes_test.cpp:411: Failure
The difference between csp_subcomponent.GetOutputVector("T_sink_in")[idx] and T_sink_in_expected[idx] is 0.12582487030903167, which exceeds 0.1, where
csp_subcomponent.GetOutputVector("T_sink_in")[idx] evaluates to 383.66582487030905,
T_sink_in_expected[idx] evaluates to 383.54000000000002, and
0.1 evaluates to 0.10000000000000001.
at index: 19
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_csp_tes_test.cpp:411: Failure
The difference between csp_subcomponent.GetOutputVector("T_sink_in")[idx] and T_sink_in_expected[idx] is 0.1270992339990471, which exceeds 0.1, where
csp_subcomponent.GetOutputVector("T_sink_in")[idx] evaluates to 383.62709923399905,
T_sink_in_expected[idx] evaluates to 383.5, and
0.1 evaluates to 0.10000000000000001.
at index: 20
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_csp_tes_test.cpp:411: Failure
The difference between csp_subcomponent.GetOutputVector("T_sink_in")[idx] and T_sink_in_expected[idx] is 0.11959526859203606, which exceeds 0.1, where
csp_subcomponent.GetOutputVector("T_sink_in")[idx] evaluates to 383.57959526859202,
T_sink_in_expected[idx] evaluates to 383.45999999999998, and
0.1 evaluates to 0.10000000000000001.
at index: 21
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_csp_tes_test.cpp:411: Failure
The difference between csp_subcomponent.GetOutputVector("T_sink_in")[idx] and T_sink_in_expected[idx] is 0.11808927687548021, which exceeds 0.1, where
csp_subcomponent.GetOutputVector("T_sink_in")[idx] evaluates to 383.51808927687546,
T_sink_in_expected[idx] evaluates to 383.39999999999998, and
0.1 evaluates to 0.10000000000000001.
at index: 22
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/shared_test/lib_csp_tes_test.cpp:411: Failure
The difference between csp_subcomponent.GetOutputVector("T_sink_in")[idx] and T_sink_in_expected[idx] is 0.1205129327666441, which exceeds 0.1, where
csp_subcomponent.GetOutputVector("T_sink_in")[idx] evaluates to 383.43051293276665,
T_sink_in_expected[idx] evaluates to 383.31, and
0.1 evaluates to 0.10000000000000001.
at index: 23
�[0;31m[ FAILED ] �[mcsp_common.TesSubcomponentCmod.Default (1 ms)
�[0;32m[----------] �[m1 test from csp_common.TesSubcomponentCmod (1 ms total)

�[0;32m[----------] �[m1 test from CMPvsamv1BatteryIntegration_cmod_pvsamv1
�[0;32m[ RUN ] �[mCMPvsamv1BatteryIntegration_cmod_pvsamv1.ResidentialDCBatteryModelPriceSignalDispatch
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/ssc_test/cmod_battery_pvsamv1_test.cpp:951: Failure
The difference between batt_q_rel.back() and 98.029 is 0.048913833456722955, which exceeds 2e-2, where
batt_q_rel.back() evaluates to 97.980086166543273,
98.029 evaluates to 98.028999999999996, and
2e-2 evaluates to 0.02.
�[0;31m[ FAILED ] �[mCMPvsamv1BatteryIntegration_cmod_pvsamv1.ResidentialDCBatteryModelPriceSignalDispatch (33889 ms)
�[0;32m[----------] �[m1 test from CMPvsamv1BatteryIntegration_cmod_pvsamv1 (33889 ms total)

�[0;32m[----------] �[m1 test from CMBattwatts_cmod_battwatts
�[0;32m[ RUN ] �[mCMBattwatts_cmod_battwatts.NoPV
/Users/imacuser/Public/Projects/GitHub/NREL/ssc/test/ssc_test/cmod_battwatts_test.cpp:183: Failure
The difference between maxCycles and 520 is 1, which exceeds 0.1, where
maxCycles evaluates to 519,
520 evaluates to 520, and
0.1 evaluates to 0.10000000000000001.
�[0;31m[ FAILED ] �[mCMBattwatts_cmod_battwatts.NoPV (271 ms)
�[0;32m[----------] �[m1 test from CMBattwatts_cmod_battwatts (271 ms total)

[sjanzou]

Note that GitHub currently has only x86_64 hosted runners, so we would need to self-host an arm64 runner for CI support or do manually before releases and patches.

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners

[brtietz]

Price signals dispatch test is a known issue filed here: #614, I don't see a reason to delay the patch for this one.

Do the other two tests include https://github.com/NREL/ssc/pull/1025/files?

[sjanzou]

@brtietz , by constraining the capacity_t::update_SOC to the min and max SOC, arm64 and x86_64 agree with 514 cycles for CMBattwatts_cmod_battwatts.NoPV.

The "Fix" was found running Windows and Mac side by side and stepping through and Windows was always getting an SOC of 14.9999999996 and the min SOC was 15. The calculated Mac value was 15. Thus, in the lib_battery_dispatch.cpp line 847, the Windows current was 6.29e-16 and was triggering a cycle and the Mac current was 0 and no cycle was triggered.

However, this does not fix the q0 difference in lib_battery_test.runTestCycleAt3C. The issue appears to be in the calculation of q0 which is dependent on qmax_thermal and qmax_lifetime.

[brtietz]

Great job finding the issue! I worry this solution could cause inconsistencies withing other variables. For example, if the battery is below the minimum SOC, that means the current was too high now or in a previous step.

Is it possible that some of the tolerances in

ssc/shared/lib_battery_capacity.cpp

Line 142 in 3d900ea

void capacity_t::check_SOC() {

are too large? That function already has the infrastructure to adjust the current for this type of issue.

[sjanzou]

Great job finding the issue! I worry this solution could cause inconsistencies withing other variables. For example, if the battery is below the minimum SOC, that means the current was too high now or in a previous step.

Is it possible that some of the tolerances in

ssc/shared/lib_battery_capacity.cpp

Line 142 in 3d900ea

void capacity_t::check_SOC() {

are too large? That function already has the infrastructure to adjust the current for this type of issue.

Agreed! I want to fix the q0 calculation upstream using qmax_thermal and qmax_lifetime and remove the SOC bound checking in capacity_t::update_SOC()

[dguittet]

Didn't see this issue before I pushed #1160

The latest Mac CI runner is now arm64, so we'd have to use macos-12 in order to test Intel Mac. The two tests that were still failing were

1. CMBattwatts_cmod_battwatts.NoPV: I didn't get down to the reason for the changes in number of cycles. I could potentially reopen the issue and try to track down the reason.

2. lib_battery_test.runTestCycleAt3C: was due to minor changes in the values so I just updated the values and/or tolerance

[sjanzou]

Didn't see this issue before I pushed #1160

The latest Mac CI runner is now arm64, so we'd have to use macos-12 in order to test Intel Mac. The two tests that were still failing were

1. CMBattwatts_cmod_battwatts.NoPV: I didn't get down to the reason for the changes in number of cycles. I could potentially reopen the issue and try to track down the reason.
2. lib_battery_test.runTestCycleAt3C: was due to minor changes in the values so I just updated the values and/or tolerance

@brtietz, @dguittet - I would suggest that we get both Intel and Apple Silicon mac runners going for the CI testing so that we can make sure there are no architecture issues. (Along with everyone running the ssc tests on Windows occasionally).

What do you think?

[brtietz]

It looks like macos-12 will be deprecated in just over a month: https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners

I'm having to read between the lines of GitHub's documentation for this a little. https://github.com/actions/runner-images/blob/main/images/macos/macos-14-arm64-Readme.md is the best resource. It looks like if we run with macos-14-arm64 and macos-14 we'll be able to do it. Do you both agree with that interpretation?

[dguittet]

I think this table is probably the best reference:

https://github.com/actions/runner-images?tab=readme-ov-file#available-images

I’ll try out adding the intel based mac runner to the CI workflow.

[dguittet]

@sjanzou @brtietz And why don't we have a Windows CI again? there are windows runners available

[brtietz]

@dguittet I haven't had time to set one up. If you have time I think it be good to have one.

[brtietz]

I think this table is probably the best reference:

https://github.com/actions/runner-images?tab=readme-ov-file#available-images

I’ll try out adding the intel based mac runner to the CI workflow.

Does NREL have one of the plans that enables the large runners? https://docs.github.com/en/actions/using-github-hosted-runners/about-larger-runners/about-larger-runners

[dguittet]

I was able to use it just now for pysam

[sjanzou]

I was able to use it just now for pysam

Awesome! Can we setup all runners for all repos? Maybe issues for each repo and we can divide the implementation?

[dguittet]

I've finished setting up Macosx intel runners for SSC, working on finishing up the Windows one, then I will create a PR.

Opened an issue: NREL/SAM#1769