Skip to content

Fix UI callback crit_load check for Custom Generation Battery configs#2049

Merged
cpaulgilman merged 5 commits into
developfrom
sam-2048-crit-load-error
Apr 4, 2025
Merged

Fix UI callback crit_load check for Custom Generation Battery configs#2049
cpaulgilman merged 5 commits into
developfrom
sam-2048-crit-load-error

Conversation

@cpaulgilman

Copy link
Copy Markdown
Collaborator

Pull Request Template

Description

Fix UI checks for critical load length to work with Custom Generation Battery configurations.

See #2048 for test procedure and sample file.

Fixes #2048

Checklist

  • requires help revision and I added that label
  • adds, removes, modifies, or deletes variables in existing compute modules
  • adds a new compute module
  • changes defaults
  • I've tagged this PR to a milestone

Reminders- this section can be deleted

Checking for PySAM Incompatible API Changes.

When do the PySAM files need to be regenerated?

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

The changes fix the bug, but I think there might be a little more work to do to clean up lingering references to generic battery. In addition to the one here, a 'find in files' search turned up suspicious references to Generic Battery in runtime/reports/pvbattery.samreport

"\tis_critical_load_tech = ( tech == 'PV Battery' \r",
"\t || tech == 'Custom Generation Battery'\r",
"\t || tech == 'Standalone Battery' \r",
"\t || tech == 'Generic Battery' \r",

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.

Is there a reason Generic Battery is still in this list?

…plate

Note that pvbattery.samreport template does not support Custom Generation Battery, so this does not affect any functionality.
@cpaulgilman

Copy link
Copy Markdown
Collaborator Author

Is there a reason Generic Battery is still in this list?

No. That was an oversight. Thanks for pointing it out.

...suspicious references to Generic Battery in runtime/reports/pvbattery.samreport

I took care of that one -- it doesn't affect any functionality because the report template is not configured for the Generic Battery configurations. Additional work on the template is required to support PDF reports for Custom Generation Battery configurations, but I did change the references to "Generic Battery" to "Custom Generation Battery" in the scripts.

@cpaulgilman cpaulgilman requested a review from brtietz April 4, 2025 17:53

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

Thanks for fixing!

@cpaulgilman cpaulgilman merged commit 2dba34f into develop Apr 4, 2025
@cpaulgilman cpaulgilman deleted the sam-2048-crit-load-error branch April 4, 2025 20:55
@cpaulgilman cpaulgilman added the added to release notes PR and/or issue has been added to release notes for a public release label Apr 7, 2025
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.

BTM Custom Generation / Battery configs critical load error

2 participants