Skip to content

Fix battery sizing behavior when switching between kWh and h definitions#720

Merged
mjprilliman merged 3 commits into
developfrom
sam-716-fix
Nov 2, 2021
Merged

Fix battery sizing behavior when switching between kWh and h definitions#720
mjprilliman merged 3 commits into
developfrom
sam-716-fix

Conversation

@mjprilliman

Copy link
Copy Markdown
Collaborator

-Make batt_bank_size a calculated variable based on either kWh UI entry or batt_bank_duration * batt_bank_power
-Update file defaults with new batt_bank_size_ui variable for displaying kWh definition in UI
-Closes #716

@brtietz brtietz 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.

Converting to a calculated variable works great and the issue in the parametrics interface has been fixed, thank you!

I worry about the large number of defaults that are being set to 0 or nan. The GUI doesn't mind, because the calculated values overwrite them immediately, but many of these are exported to the PySAM/API json (located in api/api_autogen/library/defaults), and I worry this could cause problems with those defaults. Is it possible to add the batt_bank_size_ui variable without all of these changes?

@mjprilliman

Copy link
Copy Markdown
Collaborator Author

Converting to a calculated variable works great and the issue in the parametrics interface has been fixed, thank you!

I worry about the large number of defaults that are being set to 0 or nan. The GUI doesn't mind, because the calculated values overwrite them immediately, but many of these are exported to the PySAM/API json (located in api/api_autogen/library/defaults), and I worry this could cause problems with those defaults. Is it possible to add the batt_bank_size_ui variable without all of these changes?

I believe it should be. There was an error when I first set the batt_bank_size_ui variable and ran the test script and that probably overwrote far more than what I was intending. I will take another look.

updated defaults with batt_bank_size_ui matching batt_bank_size
@mjprilliman mjprilliman requested a review from brtietz October 29, 2021 20:58

@brtietz brtietz 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 great, thank you!

@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 to me.

@mjprilliman mjprilliman merged commit 417fe21 into develop Nov 2, 2021
@mjprilliman mjprilliman deleted the sam-716-fix branch November 2, 2021 14:43
@cpaulgilman cpaulgilman added the added to release notes PR and/or issue has been added to release notes for a public release label Dec 2, 2021
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parametrics interface does not properly sizing batteries for duration

3 participants