Skip to content

add value lines to ensure reopt dispatch runs with reopt's constraints#1165

Merged
brtietz merged 15 commits into
developfrom
sam_1092_update_reopt_script
Nov 3, 2022
Merged

add value lines to ensure reopt dispatch runs with reopt's constraints#1165
brtietz merged 15 commits into
developfrom
sam_1092_update_reopt_script

Conversation

@brtietz

@brtietz brtietz commented Sep 29, 2022

Copy link
Copy Markdown
Collaborator

Pull Request Template

Description

Add value settings to the REopt sizing script such that we run REopt with the REopt friendly dispatch constraints.

Fixes #1092

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

If you have added a new compute module in a SSC pull request related to this one, be sure to check the Process Requirements.

  • My code follows the style guidelines of this project
  • [x ] I have performed a self-review of my own code
  • [x ] I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • [x ] My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • [x ] New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • [x ] I have checked my code and corrected any misspellings

@brtietz brtietz added this to the SAM Fall 2022 Release milestone Sep 29, 2022
@brtietz brtietz requested a review from cpaulgilman September 29, 2022 22:18
@brtietz brtietz self-assigned this Sep 29, 2022
@cpaulgilman

Copy link
Copy Markdown
Collaborator

@brtietz I am getting a series of "Save inputs as" dialogs saving files with invalid names like ssc-/ssc-pvsamv1.lk.lk followed by a UI callback error:

Could not evaluate callback function:call_reopt->on_change
[73]: error in call to 'reopt_size_battery()': SSC requires input 'subarray1_rear_soiling_loss', but was not found in the SAM UI or from previous simulations[5]: eval error in statement list

Also, the initial msgbox() that opens asking about changing inputs on the battery dispatch page should be made more general now that the script changes more of those check boxes:

Old:

REopt requires that battery can charge from grid.

Change Battery can Charge from Grid input on Battery Dispatch page automatically? Click No to exit optimization.

Suggested New:

REopt requires specific battery dispatch options.

Change these options automatically on the Battery Dispatch page? Click No to exit optimization.

Brian Mirletz and others added 7 commits October 11, 2022 08:54
* Remove progress bar from Battery RE Optimization RE form.

* Display message dialog box from invoke.cpp at start of fcall_reopt_size_battery().

This avoids issues with progress bar interfering with SAM UI -- not sure if UI callback messagebox with messages from REopt API will be hidden behind the dialog box.
@cpaulgilman

cpaulgilman commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator

In my tests (in sam-private so I could run the REopt API call), I am getting errors like the following:

Could not evaluate callback function:call_reopt->on_change
[67]: error in call to 'reopt_size_battery()': Invalid inputs. See 'input_errors'.
[ losses value (1.14904) in Scenario>Site>PV exceeds allowable max 0.99 ][5]: eval error in statement list

The following change to Line 5773 of invoke.cpp dividing the loss by 100 to convert from percent to decimal seems to fix the problem:

image

(annual_total_loss_percent is defined here in pvsamv1: https://github.com/NREL/ssc/blob/0ec9c8300ddc4b9287002f8a7bd5947a9f20a36a/ssc/cmod_pvsamv1.cpp#L919, losses is the DC loss input for PVWatts: https://github.com/NREL/ssc/blob/0ec9c8300ddc4b9287002f8a7bd5947a9f20a36a/ssc/cmod_pvwattsv8.cpp#L134)

I'm not sure exactly what is going on here because the REopt API inputs documentation does not list losses as an input.

@brtietz

brtietz commented Oct 26, 2022

Copy link
Copy Markdown
Collaborator Author

Oh, sorry, @Matthew-Boyd and I have a plan to fix this that he hasn't had a chance to execute yet, sorry to waste your time testing it again now.

I think that might be the link to the REopt V2 documentation - I can't find the link to the older, more comprehensive documentation, but "losses" is a variable in the code: https://github.com/NREL/REopt.jl/blob/master/src/core/pv.jl

Relatedly, I wasn't sure I should make an issue about upgrading to the V2 endpoint, since this issue exists: #1135

@cpaulgilman

Copy link
Copy Markdown
Collaborator

No need for a different issue, I added a note to #1135 to considering updates.

@brtietz

brtietz commented Nov 2, 2022

Copy link
Copy Markdown
Collaborator Author

Additional commits contain required improvements for NatLabRockies/ssc#925 to solve NatLabRockies/ssc#923. I still need to make the requested messagebox changes.

Additionally, while debugging some issues for ssc 923, I encountered the following wxWidgets error message:

image

Does this require any changes to the dialog window?

@brtietz

brtietz commented Nov 2, 2022

Copy link
Copy Markdown
Collaborator Author

@cpaulgilman Ok, this is finally ready for a second review. Please check out NatLabRockies/ssc#925 if it hasn't been merged prior to your review of this PR.

Check each required dispatch and h/kWh sizing input separately for clarity.

@cpaulgilman cpaulgilman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. I fixed the message box text, one for each input check is good for clarity. Resulting NPV is big improvement over SAM 2021.12.02, and dispatch schedule is completely different when using gen as REopt input instead of lat/lon with a loss.

@sjanzou

sjanzou commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

@cpaulgilman, @brtietz, the sam_1092_update_reopt_script pythonhandler.cpp fails to load on MacOS (found while trying to test lk pull request 36)
image

@sjanzou

sjanzou commented Nov 3, 2022

Copy link
Copy Markdown
Collaborator

@cpaulgilman, @brtietz, the sam_1092_update_reopt_script pythonhandler.cpp fails to load on MacOS (found while trying to test lk pull request 36) image

This issue is not specific to develop but is caused by a bogus python_config.json that was checked in in commit c3af95
image
This means that the current release candidate will fail if someone has not run landbosse before! We need to prevent the python_config.json to be checked in - I am going to put on the .gitignore after fixing.

@brtietz brtietz merged commit 24f383d into develop Nov 3, 2022
@brtietz brtietz deleted the sam_1092_update_reopt_script branch November 18, 2022 16:34
@cpaulgilman cpaulgilman added the added to release notes PR and/or issue has been added to release notes for a public release label Nov 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

added to release notes PR and/or issue has been added to release notes for a public release battery bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REopt dispatch should use same load/pv constraints as REopt

3 participants