Sam 313 reopt v3 for resilience

[brtietz]

Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #313

Pairs with SSC PR NREL/ssc#1078, recommend testing with SAM-private PR https://github.com/NREL/SAM-private/pull/96 as well

To test:

• REopt API call should no longer require grid charging
• New API call should be present on grid outage page for PV-battery configurations
• Sample outage to test with:
  oct_8th_outage_1wk.csv

Other notes:

• kwh/kw provided to REopt should always match the SAM case now, so removed the text about lat/lon - this will require a help update
• Reflects V3 updates from SSC pull request

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
• This change modifies variables in existing compute modules. Please see Checking for PySAM Incompatible API Changes.

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
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules
• I have checked my code and corrected any misspellings

[cpaulgilman]

1. I am getting an intermittent UI callback error -- is this being triggered by an unexpected response from the API? Clicking the button again once or twice after the error seems to get it to work.

Could not evaluate callback function:call_reopt->on_change
[103]: error: _s
[103]: error: _s
[5]: eval error in statement list

2. Is there a difference in functionality between the REopt button on the Battery Cell and System page and the one on the Grid Outage page? I find the two buttons confusing. If the two buttons do the same thing, I think I prefer a single button on the Battery Cell and System page. A alternate approach would be to have a separate optimization page because it technically affects battery size, dispatch, and grid outages.

[brtietz]

1. I saw this while testing as well and think it's an intermittent API error. Do you have any tips on how to propagate the errors at least out to these callback errors? The current error handling looks ok but is clearly missing this one.

2. The battery page does economic-only battery optimization with the PV size fixed, whereas the one on the grid outage page includes the outages and recommends a new size for PV. This is controlled with a boolean passed to invoke.cpp, so we could shift to a dialog choice easily enough if that's less confusing.

[cpaulgilman]

2. The battery page does economic-only battery optimization with the PV size fixed, whereas the one on the grid outage page includes the outages and recommends a new size for PV. This is controlled with a boolean passed to invoke.cpp, so we could shift to a dialog choice easily enough if that's less confusing.

I think I prefer a single button on the Battery Cell and System page and a dialog choice if outage is enabled. That will more clearly communicate to experienced users that there is a new feature. We can also add a sentence to the UI text explaining that considering grid outage is an option.

[cpaulgilman]

1. I saw this while testing as well and think it's an intermittent API error. Do you have any tips on how to propagate the errors at least out to these callback errors? The current error handling looks ok but is clearly missing this one.

Hard to say without knowing what the API is returning. It might be returning a non-JSON file (maybe HTML?) in which case returning a fixed number of characters of the file to display in a UI dialog may be a way to handle it. Or, retrying the API call once or twice if it doesn't return results with an error message.

[brtietz]

2. The battery page does economic-only battery optimization with the PV size fixed, whereas the one on the grid outage page includes the outages and recommends a new size for PV. This is controlled with a boolean passed to invoke.cpp, so we could shift to a dialog choice easily enough if that's less confusing.

I think I prefer a single button on the Battery Cell and System page and a dialog choice if outage is enabled. That will more clearly communicate to experienced users that there is a new feature. We can also add a sentence to the UI text explaining that considering grid outage is an option.

Addressed in latest push.

[brtietz]

1. I saw this while testing as well and think it's an intermittent API error. Do you have any tips on how to propagate the errors at least out to these callback errors? The current error handling looks ok but is clearly missing this one.

Hard to say without knowing what the API is returning. It might be returning a non-JSON file (maybe HTML?) in which case returning a fixed number of characters of the file to display in a UI dialog may be a way to handle it. Or, retrying the API call once or twice if it doesn't return results with an error message.

I think this was due to REopt V3 returning an empty dictionary if no storage is optimal, rather than size_kw = 0. I've pushed a fix to address this, can you let me know if it clears up the issue on your end?

[cpaulgilman]

2. The battery page does economic-only battery optimization with the PV size fixed, whereas the one on the grid outage page includes the outages and recommends a new size for PV. This is controlled with a boolean passed to invoke.cpp, so we could shift to a dialog choice easily enough if that's less confusing.

I think I prefer a single button on the Battery Cell and System page and a dialog choice if outage is enabled. That will more clearly communicate to experienced users that there is a new feature. We can also add a sentence to the UI text explaining that considering grid outage is an option.

Addressed in latest push.

This functionality is better. But, after testing, I think a checkbox like "Consider outage in REopt sizing" on the Battery Cell and System page would be better. It would be enabled when grid outage data is specified and disabled (and set to false) otherwise. I can add that if you agree.

[brtietz]

2. The battery page does economic-only battery optimization with the PV size fixed, whereas the one on the grid outage page includes the outages and recommends a new size for PV. This is controlled with a boolean passed to invoke.cpp, so we could shift to a dialog choice easily enough if that's less confusing.

I think I prefer a single button on the Battery Cell and System page and a dialog choice if outage is enabled. That will more clearly communicate to experienced users that there is a new feature. We can also add a sentence to the UI text explaining that considering grid outage is an option.

Addressed in latest push.

This functionality is better. But, after testing, I think a checkbox like "Consider outage in REopt sizing" on the Battery Cell and System page would be better. It would be enabled when grid outage data is specified and disabled (and set to false) otherwise. I can add that if you agree.

Yes, that sounds good. Please add it!

[cpaulgilman]

@brtietz "Consider grid outage in REopt optimization" checkbox is ready for your review.

[brtietz]

@brtietz "Consider grid outage in REopt optimization" checkbox is ready for your review.

Thank you, the checkbox looks good! I made two modifications:

1. Remove the debug "howdy doody" box
2. Add logic so that if the grid outage gets turned off, the box gets unchecked.

Can you confirm (2) is the desired behavior?

[cpaulgilman]

@brtietz "Consider grid outage in REopt optimization" checkbox is ready for your review.

Thank you, the checkbox looks good! I made two modifications:

1. Remove the debug "howdy doody" box

2. Add logic so that if the grid outage gets turned off, the box gets unchecked.

Can you confirm (2) is the desired behavior?

Oops, thanks for (1).

I went back and forth on (2), and ended up deciding not to automatically uncheck the box. The REopt call does not consider outages when the checkbox is either disabled or unchecked, which I think is clear enough, although we are not consistent with this throughout the UI. (In general, it's best not to automatically change inputs without notifying the user.)

I could see a situation where a user is trying different scenarios with and without outages on the Grid Outage page and may not notice that the box gets unchecked on the Battery Cell and System page. Or, they may be manipulating critical load and grid outage inputs in an LK script and not notice the check box being unchecked.

On the other hand, running REopt requires clicking the button on the Battery Cell and System page, so the user should see the unchecked box, so I could be convinced that automatically unchecking is the clearest option. You can decide.

[cpaulgilman]

I'll approve this pending your final decision on automatic unchecking so you can get this merged when you're ready.