Skip to content

Split ITC basis by tech part 1a: refactor single owner#1370

Merged
brtietz merged 16 commits intodevelopfrom
itc_basis_updates
Mar 16, 2026
Merged

Split ITC basis by tech part 1a: refactor single owner#1370
brtietz merged 16 commits intodevelopfrom
itc_basis_updates

Conversation

@brtietz
Copy link
Copy Markdown
Collaborator

@brtietz brtietz commented Mar 9, 2026

Pull Request Template

Description

This is the first PR in the process of updating the ITC and depreciation bases to account for OBBBA and split things by technology. This PR adds unit tests, but is otherwise just a refactor of the other code.

The near term goal is to put ITC and depreciation calculations in a class in common financial to share code between financial models prior to making further changes to behavior. Single Owner has been refactored using the new code. The other financial models have had the depreciation code extracted from their IRR target & DSCR loop to show that this code is independent of those loops (this is "new" circa 2024 - older versions of SAM this code needed to be in the loop).

Questions for this PR include:

  • Do the new variable and class names make sense?
  • Do the function scope and class scope make sense?
  • Is there anything in the wrong spot (e.g. should be in the new class, but is in SO or vice versa)?
  • Do the unit tests give you confidence that the refactor didn't break anything?
  • Any other notes before applying this to other financial models?

Happy to discuss merge strategy. (merging this to develop shouldn't break anything, but there may be a stage at which the merge makes more sense than now)

Corresponding branches and PRs:

Develop for other branches

Unit Test Impact:

Added JSON financial model unit tests that look at the combination of IRR target mode and dscr mode.

Checklist

  • I've tagged this PR to a milestone

@coveralls
Copy link
Copy Markdown

coveralls commented Mar 10, 2026

Pull Request Test Coverage Report for Build 23068101836

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 306 unchanged lines in 6 files lost coverage.
  • Overall coverage increased (+1.8%) to 58.253%

Files with Coverage Reduction New Missed Lines %
ssc/ssc/cmod_equpartflip.cpp 3 94.67%
ssc/ssc/cmod_saleleaseback.cpp 3 94.64%
ssc/ssc/cmod_levpartflip.cpp 25 94.73%
ssc/ssc/cmod_host_developer.cpp 69 93.61%
ssc/ssc/cmod_singleowner.cpp 88 92.35%
ssc/ssc/common_financial.cpp 118 79.05%
Totals Coverage Status
Change from base Build 23058170553: 1.8%
Covered Lines: 74484
Relevant Lines: 127863

💛 - Coveralls

@brtietz
Copy link
Copy Markdown
Collaborator Author

brtietz commented Mar 12, 2026

Tests added in the latest round are below. These were prioritized by commonly used features for all financial models, and less commonly used features only in single owner. Results are based off of develop as of this morning.

All Relevant financial models

  • Fixed debt percentage with and without IRR target
  • PBI for debt service
  • Bonus depreciation

Only Single Owner

  • Custom depreciation
  • Loan moratorium
  • Receivables Reserve
  • Major equipment deprecation besides 5-year MACRS
  • IBI/CBI (compliments host developer)

Additionally, added a test for Single Owner Heat.

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.

Great refactoring! The cmod_singleowner is much cleaner and the tests seem to cover everything and match with develop (before changes.

As far as GitHub Actions - I think the failing Windows test CMPvsamv1PowerIntegration_cmod_pvsamv1.UserArraySnowModel should be made in a separate pull request as this one continues to pass/fail on Windows intermittently. Works on other runners consistently.

I also, went through and updated the SAM itc_basis_updates branch to work with the ortools merge and updated defaults and results and went through to check for other tests to add for cmod_singleowner - I did not find any.

Great work - very meticulous!

Test("singleowner", file_inputs, file_outputs, compare_number_variables, compare_array_variables);
}

TEST_F(CmodSingleOwnerTest, CustomDepr) {
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 adding all of these!

@brtietz
Copy link
Copy Markdown
Collaborator Author

brtietz commented Mar 16, 2026

CMPvsamv1PowerIntegration_cmod_pvsamv1.UserArraySnowModel

Thanks for the review! I created an issue for this here: #1372

@brtietz brtietz merged commit 2ea4044 into develop Mar 16, 2026
13 of 17 checks passed
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.

3 participants