Skip to content

Add "Allen method" to Subhourly Clipping Correction Option#1861

Merged
cpaulgilman merged 7 commits into
developfrom
sam-1850-allen-clipping-correction
Oct 3, 2024
Merged

Add "Allen method" to Subhourly Clipping Correction Option#1861
cpaulgilman merged 7 commits into
developfrom
sam-1850-allen-clipping-correction

Conversation

@cpaulgilman

@cpaulgilman cpaulgilman commented Oct 1, 2024

Copy link
Copy Markdown
Collaborator

Pull Request Template

Description

  • Change check box label in UI
  • Revise Help to add references and clarify difference between hourly and subhourly simulations.

Note Help revisions for the "Losses" topic will be committed separately before the Fall 2024 release. Please review and comment on following screenshot.

image

Fixes #1850 (issue)

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 cpaulgilman added the UI User interface issue that applies across performance and financial models label Oct 1, 2024
@cpaulgilman cpaulgilman added this to the SAM Fall 2024 Release milestone Oct 1, 2024
@cpaulgilman cpaulgilman self-assigned this Oct 1, 2024
@mjprilliman

Copy link
Copy Markdown
Collaborator

@cpaulgilman is it ok to add to this branch to add a switch between Allen and Walker methods? It would likely be a dropdown with Allen method and Walker method as the text in the dropdown menu.

@cpaulgilman

Copy link
Copy Markdown
Collaborator Author

@cpaulgilman is it ok to add to this branch to add a switch between Allen and Walker methods? It would likely be a dropdown with Allen method and Walker method as the text in the dropdown menu.

Yes -- consider radio buttons in case dropdown suggests preference for one option over the other. Also, is there a way to differentiate the methods with a descriptive term instead of the developer's name?

@mjprilliman

Copy link
Copy Markdown
Collaborator

@cpaulgilman is it ok to add to this branch to add a switch between Allen and Walker methods? It would likely be a dropdown with Allen method and Walker method as the text in the dropdown menu.

Yes -- consider radio buttons in case dropdown suggests preference for one option over the other. Also, is there a way to differentiate the methods with a descriptive term instead of the developer's name?

@cpaulgilman I would appreciate notes on the wording here. We can provide references to Allen and Walker in the documentation.

image

Walker method: https://www.nrel.gov/docs/fy20osti/76859.pdf

@cpaulgilman

Copy link
Copy Markdown
Collaborator Author

@mjprilliman Thanks for adding the Walker method. I've redesigned the UI and revised Help:

image

image

@mjprilliman

Copy link
Copy Markdown
Collaborator

@mjprilliman Thanks for adding the Walker method. I've redesigned the UI and revised Help:

image

image

Looks great Paul! My only note would be that only the Allen method I would describe as empirical, so I would probably remove that adjective there.

@cpaulgilman

Copy link
Copy Markdown
Collaborator Author

Looks great Paul! My only note would be that only the Allen method I would describe as empirical, so I would probably remove that adjective there.

Good catch. I'll remove "empirical" from the general description.

@cpaulgilman cpaulgilman merged commit 8b5829f into develop Oct 3, 2024
@cpaulgilman cpaulgilman added the added to release notes PR and/or issue has been added to release notes for a public release label Dec 11, 2024
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 UI User interface issue that applies across performance and financial models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add details on subhourly clipping loss correction

2 participants