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

2023 defaults updates #1581

Merged
merged 30 commits into from Nov 28, 2023
Merged

2023 defaults updates #1581

merged 30 commits into from Nov 28, 2023

Conversation

brtietz
Copy link
Collaborator

@brtietz brtietz commented Nov 16, 2023

Update defaults for the 2023 release. Changes include:

  1. Update PV and battery+x costs to align with 2023 PV+battery benchmark
  2. Update standalone battery and wind costs to align with 2023 ATB
  3. Update financials to align with 2023 ATB
  4. Fix Known defaults issues for the fall release #1471
  5. Update PPA prices to produce positive NPVs with the above (at least for single owner)
  6. Update computed values for PySAM with automated override script

All references pushed to SAM-documentation.

The following have not been updated:

  1. Host developer sizes (pending fix to Load powerflow Metrics work for PV+Battery, but not hybrids ssc#1092)
  2. PySAM defaults/API autogen files

@brtietz brtietz added this to the SAM Fall 2023 Release milestone Nov 16, 2023
@brtietz
Copy link
Collaborator Author

brtietz commented Nov 16, 2023 •
edited

Doing some initial QC, it looks like step 6 above didn't have the intended effect, many of the total installed cost and construction financing cost fields still need to be updated.

@sjanzou Are any changes to https://github.com/NREL/SAM/blob/develop/deploy/overwrite_all_defaults.lk necessary to ensure the callbacks and equations run?

@cpaulgilman
Copy link
Collaborator

Fixes #1405

cpaulgilman
cpaulgilman approved these changes Nov 16, 2023
View reviewed changes
Copy link
Collaborator

@cpaulgilman cpaulgilman left a comment

Choose a reason for hiding this comment

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

Looks good. I fixed one issue with calculated PV capacity for hybrid config, and did a bunch of cleanup of default collapsible_var.

@sjanzou
Copy link
Collaborator

sjanzou commented Nov 17, 2023

Doing some initial QC, it looks like step 6 above didn't have the intended effect, many of the total installed cost and construction financing cost fields still need to be updated.

@sjanzou Are any changes to https://github.com/NREL/SAM/blob/develop/deploy/overwrite_all_defaults.lk necessary to ensure the callbacks and equations run?

The update was made to remove the “Retired” technology that was failing. All others were updating without issue.

@brtietz
Copy link
Collaborator Author

brtietz commented Nov 17, 2023

Doing some initial QC, it looks like step 6 above didn't have the intended effect, many of the total installed cost and construction financing cost fields still need to be updated.
@sjanzou Are any changes to https://github.com/NREL/SAM/blob/develop/deploy/overwrite_all_defaults.lk necessary to ensure the callbacks and equations run?

The update was made to remove the “Retired” technology that was failing. All others were updating without issue.

Does it work on Mac? It hasn't been working for me on Windows.

@sjanzou
Copy link
Collaborator

sjanzou commented Nov 17, 2023

Doing some initial QC, it looks like step 6 above didn't have the intended effect, many of the total installed cost and construction financing cost fields still need to be updated.
@sjanzou Are any changes to https://github.com/NREL/SAM/blob/develop/deploy/overwrite_all_defaults.lk necessary to ensure the callbacks and equations run?

The update was made to remove the “Retired” technology that was failing. All others were updating without issue.

Does it work on Mac? It hasn't been working for me on Windows.

Sorry, I thought you were talking about the lk script. The reason that the calculated values are not updating is due to the fic for issue #1456 … the equations are no longer run when setting the configuration, only when the case is loaded in SAM #1583 so that the version upgraded will be called before equations are evaluated. We can modify the lk script to load each of the configurations which will update the defaults in whatever branch SAM_1583 and SAM_1456 are merged.

@brtietz
Copy link
Collaborator Author

brtietz commented Nov 17, 2023

Sorry, I thought you were talking about the lk script. The reason that the calculated values are not updating is due to the fic for issue #1456 … the equations are no longer run when setting the configuration, only when the case is loaded in SAM #1583 so that the version upgraded will be called before equations are evaluated. We can modify the lk script to load each of the configurations which will update the defaults in whatever branch SAM_1583 and SAM_1456 are merged.

Ah, ok! I'm planning to merge this into develop after test_script_ow.lk finishes running locally, which will bring SAM_1583 into develop and we can fix the script there. Thanks!

@sjanzou
Copy link
Collaborator

sjanzou commented Nov 19, 2023 •
edited

GitHub Actions failing tests caused by the MSLF/SO configuration run
image

Runs fine in SAM but not using the test_script_ow.lk lk commands

sjanzou and others added 2 commits November 19, 2023 10:42
@sjanzou @brtietz
fixes issue #1596 - update calculated value using overwrite_all_defau…
efce165 
…lts.lk (#1597)

* Working on #1596

* Update fcall_overwrite to evaluate equations before saving.

* Fix missing "load" for battery dispatch for hybrid host developer configurations

* code cleanup

* add battery_load default to compliment changes to startup.lk

---------

Co-authored-by: Brian Mirletz <brian.mirletz@nrel.gov>
@sjanzou
#1598 overwrite_defaults to update calculated values correctly for hy…
e98f350 
…brids

Update CaseEvent class to properly set ndxHybrid for CaseWindow::OnCaseEvent handling.
@sjanzou sjanzou mentioned this pull request Nov 20, 2023
Closed
sjanzou
sjanzou approved these changes Nov 26, 2023
View reviewed changes
Copy link
Collaborator

@sjanzou sjanzou left a comment

Choose a reason for hiding this comment

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

test_script_ow.lk and overwrite_all_defaults.lk passing now.
Defaults look good!

@sjanzou sjanzou mentioned this pull request Nov 28, 2023
Merged
@brtietz brtietz merged commit e29f61b into develop Nov 28, 2023
4 checks passed
@brtietz brtietz mentioned this pull request Nov 28, 2023
Closed
@cpaulgilman cpaulgilman added the added to release notes PR and/or issue has been added to release notes for a public release label Nov 30, 2023
@brtietz brtietz deleted the 2023_defaults_updates branch December 13, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjanzou sjanzou sjanzou approved these changes

@cpaulgilman cpaulgilman cpaulgilman approved these changes

Assignees
No one assigned
Labels
added to release notes PR and/or issue has been added to release notes for a public release
Projects
None yet
Milestone
SAM Fall 2023 Release
Development

Successfully merging this pull request may close these issues.

Known defaults issues for the fall release
4 participants
@brtietz @cpaulgilman @sjanzou @janinefreeman