Size the collector loop mass flow based on n_collectors#1004
Merged
Conversation
cpaulgilman
reviewed
Mar 9, 2023
cpaulgilman
left a comment
Collaborator
There was a problem hiding this comment.
@Matthew-Boyd I removed a user input "Total system flow rate" that set the value of the SSC input mdot to resolve NatLabRockies/SAM#1153.
Looking at that now, it seems like we should restore that input. Removing it was the result of some internal discussions that concluded that SSC was not using the input.
Contributor
Author
|
@cpaulgilman It's definitely using that input and bad behavior occurs when the number of collectors is increased. My opinion is that the mass flow through the collector array should either be constant and proportional to the number of collectors and their respective test flow rates, or it should be automatically adjusted within an operating window to control the outlet temperature. Currently in the code it is a constant value irrespective of the number of collectors, which doesn't make sense. I also don't think the user should need to calculate this, nor should it be calculated in a UI equation because it won't translate to scripting or PySAM. |
Collaborator
|
@Matthew-Boyd That makes sense. |
cpaulgilman
approved these changes
Mar 9, 2023
cpaulgilman
added a commit
that referenced
this pull request
Jun 10, 2023
* 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>
brtietz
pushed a commit
that referenced
this pull request
Oct 10, 2023
* 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) * 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 internal resistance to voltage table calculations * Update voltage equations and tests to account for zero current at full battery, new resistance in table tests * update resilience tests and fix voltage at 100 percent dod --------- Co-authored-by: Steven Janzou <sjanzou@gmail.com> Co-authored-by: Paul Gilman <cpaulgilman@gmail.com> Co-authored-by: Matthew Boyd <30417543+Matthew-Boyd@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the collector loop solar water heating mass flow as this is not currently dependent on the number of collectors, with the value currently being a constant sized for two collectors.
@cpaulgilman Can you please make sure I'm not misunderstanding what is going on here? I think the UI originally sized this parameter but no longer does.
Closes #1003