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.

Sign up for GitHub

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

Jump to bottom

Different results for batterystateful in amd64 and aarch64 #1023

Closed
cpaulgilman opened this issue May 11, 2023 · 0 comments · Fixed by #1025
Closed

Different results for batterystateful in amd64 and aarch64 #1023

cpaulgilman opened this issue May 11, 2023 · 0 comments · Fixed by #1025
Assignees
@sjanzou
Milestone
2022.11.21 Patch 2

Comments

@cpaulgilman
Copy link
Collaborator

cpaulgilman commented May 11, 2023 •
edited
Loading

A PySAM 4.1.0 script running the BatteryStateful module generates different results in Windows and Mac. Test results by @sjanzou:

I was able to reproduce the result Anton received on his Mac on my M1 Max MacBook Pro and on a Debian 11 aarch64 docker container.

On the same Mac using a Debian 11 amd64 (x86_64) docker container, I was able to reproduce your Windows results shown on the forum.

Sample PySAM script here: https://sam.nrel.gov/forum/attachment/1514

See https://sam.nrel.gov/forum/forum-sdk/4212
@cpaulgilman cpaulgilman added this to the 2022.11.21 Patch 2 milestone May 11, 2023
@brtietz brtietz removed their assignment May 16, 2023
sjanzou added a commit that referenced this issue May 19, 2023
@sjanzou
Create ssc test to match python script issue in #1023
a8eb262
@sjanzou sjanzou linked a pull request May 21, 2023 that will close this issue
Fixes #1023 #1025
Merged
sjanzou added a commit that referenced this issue May 23, 2023
@sjanzou
Merge pull request #1025 from NREL/ssc_1023
f8d64af 
Fixes #1023
cpaulgilman added a commit that referenced this issue Jun 10, 2023
@cpaulgilman @sjanzou
@Matthew-Boyd @brtietz @mjprilliman @tyneises
Merge SSC patch into develop (#1033)
c5d0412 
* Address loss diagram issue for POA reference cell in SAM issue 1366

* Working without pressure column 

Need to remove some modifications in cmod_pvsamv1 - set to SAM_1366 branch

* Reset POA_P mode - uses decomposition with and without pressure column

* Add wfpoa to reported outputs to facilitate troubleshooting

* Size the collector loop mass flow based on n_collectors (#1004)

* create new battery reopt sizing function. move variable mapping functions to vartab to make them accessible to both functions

* Remove redundant variables between PV and battery sizing functions. PV now calls battery sizing

* Update to GitHub Actions to latest Ubuntu

Ubuntu-18.04 deprecated as of 4/2023 https://github.blog/changelog/2022-08-09-github-actions-the-ubuntu-18-04-actions-runner-image-is-being-deprecated-and-will-be-removed-by-12-1-22/

* add comment

* Initial sg2 implementation, currently debugging

* Commented out unused code sections, prepped for speed testing

* update ssc version for testing and benchmarking

* Removed commented lines, update test results (irrad tests, pvwatts
tests)

* Updated test results

* Re-run PVWatts cmod tests for different DC-AC ratio, updated remaining
tests

* Allow more flexible SAM CSV for solar weather file format (#1017)

- Allow files that have minimum required metadata, regardless of number of columns in metadata header rows.
- Add Version to list of metadata columns to support NSRDB version numbers.
- Test with files from NSRDB, PVWatts API, and hand-edited to have different metadata row lengths.

* Update to fix broken MacOS build

"Call to undeclared function 'gettimeofday'; ISO C99 and later do not support implicit function declarations" compile error

* Throw an error if duplicate entries in the voltage table produce zero slope

* adjust voltage profile to comply with new restrictions

* Add errors for length mismatch between critical load and electric load (#1021)

* Add errors for length mismatch between critical load and electric load

* Review and revise battery error messages.

---------

Co-authored-by: Paul Gilman <cpaulgilman@gmail.com>

* update documentation for standalone battery params

* fix fabs error on linux

* Add test for reopt sizing post for standalone battery

* Better explanation of this function's limitations

* Fix day_of function to return 0-364 rather than 0-6

* Update tests

* Setup test for ssc issue 1023

* Create ssc test to match python script issue in #1023

* additional test outputs and verify "good" results on amd64 Windows 11

* fix issue on arm64 in lib_battery_voltage

TODO - test on Windows and regenerate pysam and test on arm64, aarch64 and amd64

* update test not to write out all values and check final iteration

* Update test comment for future reference

* Turn all ssc tests back on for pull request #1025

* Remove sf_adjustment_factors class and use adjustment_factors class for sf_adjustment

* Remove unused callbacks causing Github Actions to fail

* Update CMakeLists.txt to skip removed include files for Github Actions

* Restore commented out files

* Restore all per patch and build locally on Linux

* Remove extra qualification on size() function causing platforms other than Windows to fail

Note - no error message in Github Actions log....

* Give price signals dispatch info about real time system power and load for dispatch for apples to apples comparison of forecast error

* Update integration tests with new cycling value for look behind case

* Fix conflict for merge of patch into develop

Remove duplicate definition of size() in adjustment factors class.

* Fix broken tests

Run ssc/tes/\input_json/ssc_test_save_as_JSON_test.sam in latest SAM and generate updated json files for 'save_as_JSON_test'.
Run ssc/test/input_json/TechnologyModel/pvwattsv8/generate_json.lk for all CmodPVWattsv8Test.DCACRatio* tests

---------

Co-authored-by: Steven Janzou <sjanzou@gmail.com>
Co-authored-by: Matthew Boyd <30417543+Matthew-Boyd@users.noreply.github.com>
Co-authored-by: Brian Mirletz <brian.mirletz@nrel.gov>
Co-authored-by: Matt Prilliman <Matthew.Prilliman@nrel.gov>
Co-authored-by: Steven Janzou <steven@janzouconsulting.com>
Co-authored-by: Matt Prilliman <54449384+mjprilliman@users.noreply.github.com>
Co-authored-by: tyneises <ty.neises@nrel.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sjanzou sjanzou

Labels
None yet
Projects
None yet
Milestone
2022.11.21 Patch 2
Development

Successfully merging a pull request may close this issue.

Fixes #1023 NREL/ssc
3 participants
@brtietz @sjanzou @cpaulgilman