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.

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

Sam 1064 correct monthly curtailment #1068

Merged
merged 4 commits into from
Oct 31, 2023

Conversation

brtietz
Copy link
Collaborator

@brtietz brtietz commented Oct 18, 2023

Fixes NREL/SAM#1064

Recalculates monthly energy in cmod grid & moves annual outputs to the attic to avoid confusion

Generic system with a constant generation profile makes a good test. A grid connection limit of 1 kW should result in monthly energy equal to the number of hours in each month. Sample file with that configuration:
monthly_values_interconnection_limits.zip

@brtietz brtietz added the bug label Oct 18, 2023
@brtietz brtietz added this to the SAM Fall 2023 Release milestone Oct 18, 2023
@brtietz brtietz self-assigned this Oct 18, 2023
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.

The changes look fine and thanks for putting annualoutput in the attic...

However, the geothermal test is failing since the compute module has a monthly output and the grid monthly energy seems to be overwriting with length 12 instead of 12*25 = 300 months in the analysis period.

Please check the test and update, thanks!

image

@brtietz
Copy link
Collaborator Author

brtietz commented Oct 19, 2023

@sjanzou, @mjprilliman, and @cpaulgilman This strikes me as an odd convention from cmod_geothermal. In particular:

  1. It's the only compute module where monthly_energy isn't length 12
  2. It's using an array where other technologies would use a matrix and the year-month structure

My inclination is to revise cmod_geothermal such that monthly_energy follows the convention of other compute modules (length 12, year 1 only) and then implement a new variable that displays the lifetime data without curtailment to preserve the current output.

Does this break anything that you're aware of downstream? If not, what are your preferences between maintaining the current array structure versus shifting to a matrix with the year-month structure? Given answers to those I can propose some variable names.

@mjprilliman
Copy link
Collaborator

@sjanzou, @mjprilliman, and @cpaulgilman This strikes me as an odd convention from cmod_geothermal. In particular:

  1. It's the only compute module where monthly_energy isn't length 12
  2. It's using an array where other technologies would use a matrix and the year-month structure

My inclination is to revise cmod_geothermal such that monthly_energy follows the convention of other compute modules (length 12, year 1 only) and then implement a new variable that displays the lifetime data without curtailment to preserve the current output.

Does this break anything that you're aware of downstream? If not, what are your preferences between maintaining the current array structure versus shifting to a matrix with the year-month structure? Given answers to those I can propose some variable names.

I'm not opposed to changing the variable name. I'll have to check the downstream changes but I don't anticipate it being too much difficulty. Monthly energy output is an important metric for geothermal (more so than hourly typically) so I'm hesitant to put it into a matrix structure. I think a newly named array variable would be the best path forward.

@brtietz
Copy link
Collaborator Author

brtietz commented Oct 19, 2023

Thanks! A few name options:

  • monthly_energy_lifetime
  • monthly_energy_gross
  • monthly_energy_pre_curtailment

Preferences?

@cpaulgilman
Copy link
Collaborator

Thanks! A few name options:

* monthly_energy_lifetime

* monthly_energy_gross

* monthly_energy_pre_curtailment

Preferences?

I think monthly_energy_lifetime is the least mysterious.

@brtietz brtietz requested a review from sjanzou October 26, 2023 15:46
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. Tested in SAM with Generic / Single Owner and Geothermal / Single Owner, and reviewed monthly energy variable names in LK script.

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.

Thanks for updating the tests! Changes look good!

@sjanzou sjanzou merged commit 92e232c into develop Oct 31, 2023
4 checks passed
@sjanzou sjanzou deleted the sam_1064_correct_monthly_curtailment branch October 31, 2023 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Grid interconnection limit doesn't impact monthly_energy
4 participants