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

along with ssc branch SAM_675, this fixes SAM issue 675 #1043

Merged
merged 2 commits into from May 19, 2022
Merged

Conversation

sjanzou
Copy link
Collaborator

@sjanzou sjanzou commented May 17, 2022

No description provided.

@sjanzou
Copy link
Collaborator Author

sjanzou commented May 17, 2022

This pull request goes with NREL/ssc#817
Note that saving and reloading with less than 25 year analysis period fixed in SAM pull request 1041 #1041

Files for testing and details of fix:
SAM_675.zip

@sjanzou sjanzou linked an issue May 17, 2022 that may be closed by this pull request
Copy link
Collaborator

@brtietz brtietz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix makes sense - but why the big increases in LCOE in the test results?

@cpaulgilman
Copy link
Collaborator

cpaulgilman commented May 18, 2022

This is a bit of a nit-pick, and maybe not worth the time to fix, but would be good for the order of Mode options to be logical:

image

Perhaps:

Subhourly
Hourly
Daily
Weekly
Monthly
Annual

@sjanzou
Copy link
Collaborator Author

sjanzou commented May 19, 2022

Fix makes sense - but why the big increases in LCOE in the test results?

@brtietz, running the patch 1 release and the SAM_675 branch shows that all the Windows defaults results match except PV Battery / SO which are close:
image

So, I assume that the previous test_results_win64.csv were somehow corrupted by a previous merge.

@sjanzou
Copy link
Collaborator Author

sjanzou commented May 19, 2022

This is a bit of a nit-pick, and maybe not worth the time to fix, but would be good for the order of Mode options to be logical:

image

Perhaps:

Subhourly
Hourly
Daily
Weekly
Monthly
Annual

@cpaulgilman, I have created a separate issue #1046 for rearranging as the original control had weekly and annual as optional mode selections and the enums for the list do not line up.

@sjanzou sjanzou merged commit 166a184 into patch May 19, 2022
@sjanzou sjanzou deleted the SAM_675 branch May 19, 2022 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Weekly Price Timeseries Extrapolation Missing Day
3 participants