Skip to content

Fix wind adjustment factor double-counting#1284

Merged
cpaulgilman merged 3 commits intopatchfrom
ssc-1283-wind-adj-factors
Feb 26, 2025
Merged

Fix wind adjustment factor double-counting#1284
cpaulgilman merged 3 commits intopatchfrom
ssc-1283-wind-adj-factors

Conversation

@cpaulgilman
Copy link
Copy Markdown
Collaborator

Fix #1283

To test, run a default Wind / No Financial case with and without time series adjustment factors. The difference between the two runs should be the adjustment factor applied once, not twice as reported in #1283.

image

The adjustment factors should work correctly for the following combinations:

  • Hourly weather file with hourly losses
  • Subhourly weather file with subhourly losses
  • Weibull distribution with hourly losses
  • Wind resource probability table with hourly losses
  • Constant loss wake model option with hourly or subhourly simulations/losses

A simulation error should be generated with the wrong combination of wind data time step and loss time step.

The attached file has .sam file with cases set up for these tests and a 15-minute wind file:
windpower-adj-test.zip

This PR also fixes time series adjustment losses (haf) indexed to hr instead of time step i: Subhourly loss supported as of NatLabRockies/SAM#1164.

Time series adjustment factors (haf) are no longer limited to hourly.
@cpaulgilman cpaulgilman added this to the 2024 Release Patch 1 milestone Feb 26, 2025
@cpaulgilman cpaulgilman self-assigned this Feb 26, 2025
Copy link
Copy Markdown
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.

This works but the cmod_windpower compute module is extremely confusing... why is there a return statement in and if block on line 424? Seems like the Weibull model and the Wake model should be separated with an if else structure for easier debugging.

Also, the error message when using the 15min resource file and the hourly adjustment factors is a little confusing
image

Would it be clearer to use "resource file time steps" (for resource file inputs (wind_resource_model_choice = 0) instead of "simulation time steps"?

Also, same issue with the "wind resource probability table" case in the supplied "windpower-adj-test.sam" file:
image

There is an explicit simulation time steps of 8760 using the wind resource probability table option (wind_resource_model_choice = 2) on line 488 after the adjustment factors are setup:
image

For the Weibull resource input option (wind_resource_model_choice = 1), the nsteps is explicitly set to 8760 on line 394 in cmod_windpower:
image

@cpaulgilman
Copy link
Copy Markdown
Collaborator Author

...but the cmod_windpower compute module is extremely confusing... why is there a return statement in and if block on line 424? Seems like the Weibull model and the Wake model should be separated with an if else structure for easier debugging.

I agree. I implemented this solution in a way that did not require restructuring the compute module. I think the double-counting of adjustment factors was originally caused by the confusing structure.

@janinefreeman
Copy link
Copy Markdown
Collaborator

All scenarios work well for me. Agree with @sjanzou that changing the error label to "resource file time steps" might be helpful, but not totally opposed to keeping "simulation time steps" in those error messages.

@sjanzou agree that the wind compute module can be confusing and might do good with a refactor, but unfortunately we don't have funding for that right now. I think the return in the if chunk on line 424 can stay for now. ;)

@janinefreeman janinefreeman removed the request for review from mjprilliman February 26, 2025 16:10
@cpaulgilman
Copy link
Copy Markdown
Collaborator Author

There is an explicit simulation time steps of 8760 using the wind resource probability table option (wind_resource_model_choice = 2) on line 488 after the adjustment factors are setup...

Yes. The number of time steps is set in several places and determined from the weather file in other places. Some functions appear to require 8760 time steps (e.g., Weibull distribution and wind resource probability table and possibly some wake model options), and others work with either hourly or subhourly time steps. The compute module should be restructured so that all functions support subhourly time steps to work with hourly and subhourly adjustment losses, and to work with subhourly financial model features like market prices, etc.

@cpaulgilman
Copy link
Copy Markdown
Collaborator Author

cpaulgilman commented Feb 26, 2025

Would it be clearer to use "resource file time steps" (for resource file inputs (wind_resource_model_choice = 0) instead of "simulation time steps"?

I'll revise them to make them less confusing for users.

@cpaulgilman cpaulgilman merged commit 2063a47 into patch Feb 26, 2025
8 checks passed
@cpaulgilman cpaulgilman deleted the ssc-1283-wind-adj-factors branch February 26, 2025 17:25
@brtietz brtietz modified the milestones: 2024 Release Patch 1, SAM 2025 release patch 1, SAM Spring 2025 Release Mar 7, 2025
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.

Windpower module applies HAF twice

4 participants