Skip to content

Conversation

softwareengineerprogrammer
Copy link
Owner

@softwareengineerprogrammer softwareengineerprogrammer commented Mar 10, 2025

Description of Changes

Revenue & Cashflow Profile periods (aka "Year Since Start") are now displayed in alignment with the convention given in the NREL manual as discussed in NREL#344:

image

revenue-profile-period-correction_profile-start-and-end-highlighted

As highlighted in the first row, the first entry is now correctly labeled as 0 years since start. The last row is also highlighted because this change also fixes the issue with last year of R&C profile not being included: NREL#350

Testing & Verification

Testing & verification of fix:

  1. Regenerated all example files per standard procedure. Note that for all example files, only the R&C profile changed and not any of the derived/corresponding economic metrics like NPV/IRR/etc. This is because those metrics were already being calculated with the correct underlying data/conventions and this issue was limited to a matter of displaying the profile correctly and conventionally.
  2. Manually verified/sanity-checked select example results.
  3. Systematically checked by writing a 'one-time' unit test to assert parity between the updated Revenue & Cashflow Profiles and the pre-fix versions. This check ensured that the only changes to the profile were the year indexing and inclusion of final year (as opposed to an unintended change to the profile values themselves). Successful Actions run: https://github.com/softwareengineerprogrammer/GEOPHIRES/actions/runs/13814321818
  4. Unit test profile with multiple construction years (total capex evenly spread over construction years)
  5. Verified Revenue & Cashflow Profile graph renders correctly/appropriately in web interface: image
  6. Verified R&C profile in CSV downloaded from web interface is correct: image

TODO & Follow-Up Tasks

TODO:

  1. Verify additional construction year values: https://github.com/softwareengineerprogrammer/GEOPHIRES/pull/54/files#diff-01c5adc6b171191b928e59a6206cad4a36e9f2cec9c939e60b4819d518e36664R714-R747
  2. v3.8 changelog entry: 89c2941

Follow-up PR: #53

…value = 0. Fix issue with last year of R&C profile not being included. NREL#350
@keithtelliott
Copy link

Way to document via a unit test!

@softwareengineerprogrammer
Copy link
Owner Author

Way to document via a unit test!

@keithtelliott FYI if you are referring to the number/size of diffs on tests/examples/*.out - this is because we maintain the expected outputs from every GEOPHIRES example in source in tests/examples. We compare the expected result as defined in source with the actual result we get from running GEOPHIRES in the GeophiresXTestCase.test_geophires_examples unit test. (This testing is a big part of how we achieve 80+% test coverage.)

We therefore have to update the corresponding tests/examples/*.out files when we make a change that affects expected example results. Because this PR affects the entire revenue profile in every example, we end up with a sizeable diff. Fortunately, we do not have to make these updates by hand. Instead, when GeophiresXTestCase.test_geophires_examples fails, it generates and outputs a command the developer can run to regenerate failing example tests (assuming the failures are expected due to calculation updates): https://github.com/NREL/GEOPHIRES-X/blob/d40a516c7c67769e32387b69fe185a3f2b3b6451/tests/test_geophires_x.py#L228-L241

(Filed an issue to add this explanation to our development instructions: NREL#355)

@softwareengineerprogrammer softwareengineerprogrammer changed the title Revenue Profile Period Correction Revenue Profile Period Correction [v3.8] Mar 12, 2025
@softwareengineerprogrammer softwareengineerprogrammer changed the title Revenue Profile Period Correction [v3.8] Revenue Profile Period & Final Year Correction [v3.8] Mar 12, 2025
@softwareengineerprogrammer softwareengineerprogrammer merged commit 4577bcf into main Mar 12, 2025
24 checks passed
@softwareengineerprogrammer softwareengineerprogrammer deleted the revenue-profile-period-correction branch May 1, 2025 17:43
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.

2 participants