Multi-year sell-price in ProFAST NPV by elenya-grant · Pull Request #799 · NatLabRockies/H2Integrate

elenya-grant · 2026-07-06T20:44:31Z

Multi-year sell-price in ProFAST NPV

Updated ProFastNPV so that the commodity sell price input is the same length as the plant life. This enables users to be able to set a sell price for the NPV calculation that is different per year.

Shoutout to @vijay092 for bringing this up!

Question/Note: until PR #774 is merged, this change would prevent someone from being able to connect a single value to the sell price input (such as [finance_subgroup_electricity, finance_subgroup_electricity_NPV, LCOE, .commodity_sell_price_electricity]). If we want to support this type of connection before #774 is merged in, then I could change the logic (it will be not as clean) so that the sell-price can be input with either a shape of 1 or a shape equal to the plant life. Please let me know whether the handling to enable this type of connection would be valuable in this PR, or if that doesn't need to be included here because it'll be easily enabled with PR #774.

Section 1: Type of Contribution

Section 2: Draft PR Checklist

Open draft PR
Describe the feature that will be added
Fill out TODO list steps
Describe requested feedback from reviewers on draft PR
Complete Section 7: New Model Checklist (if applicable)

TODO:

Depends on reviewer feedback

Type of Reviewer Feedback Requested (on Draft PR)

Structural feedback:
see section 1
Implementation feedback:

Other feedback:

Section 3: General PR Checklist

Section 4: Related Issues

Section 5: Impacted Areas of the Software

Section 5.1: New Files

N/A

Section 5.2: Modified Files

h2integrate/finances/profast_npv.py
- ProFastNPV.setup(): made the length of the commodity sell price input equal to plant life. Also added more error messages when checking the commodity sell price input.
- ProFastNPV.compute(): added padding to the price profile input so that its the proper length for the profast cash_flow calculation
h2integrate/finances/test/test_profast_npv.py: added two tests:
- test_profast_npv_multi_year_sell_price: tests that NPV is output as expected if a list of sell prices are input
- test_profast_npv_multi_year_error: tests that an error is thrown if sell price is the wrong length

Section 6: Additional Supporting Information

Section 7: Test Results, if applicable

…ar_sell_price

vijay092

Thank you so much, Elenya!! This looks great to me! I had some minor comments about inflation rate.

vijay092 · 2026-07-08T17:41:52Z


+        if isinstance(self.commodity_sell_price, float | int):
+            if self.commodity_sell_price is None:
+                raise ValueError("commodity_sell_price is missing as an input")


None is neither a float nor an int so if the self.commodity_sell_price is actually None, it won't enter the previous condition.

Also worth adding as a test

good catch! thank you!

I fixed the logic and added in a test for it test_profast_npv_missing_sell_price

vijay092 · 2026-07-08T18:40:52Z

+        "finance_parameters": {"model_inputs": profast_inputs_no2},
+    }
+    pf = ProFastNPV(
+        driver_config={},


Both test fixtures use inflation_rate: 0.0. Can you add a case with nonzero inflation to confirm the per-year array isn't getting escalated a second time by ProFAST's own calculation? If users are meant to supply already-nominal per-year prices, we may need to force infation_rate = 0 when an array is passed.

Also, could we clarify in the docs whether commodity_sell_price should be real or nominal dollars? Per our discussion with @jaredthomas68, we need to set nominal prices with inflation_rate = 0 (since ProFAST would otherwise apply its own escalation on top). Worth spelling that out explicitly.

Can you add a case with nonzero inflation to confirm the per-year array isn't getting escalated a second time by ProFAST's own calculation

I played around with ProFAST using a 2% inflation rate and have some info in case its useful:

pf.cash_flow(price=0.07) and pf.cash_flow(price=[0.7]*30) result in the same NPV

The NPV with an inflation rate of 2% is about 2.2x the NPV with 0 inflation (both using a sell price of 0.7)

I think ProFAST does escalate the sell price, it basically does this:

analysis_length = plant_life + (installation_period_months/12) analysis_years = np.arange(0, analysis_length + 1) sales_price = commodity_sell_price*(1.0+inflation)**(analysis_years-1) revenue_from_sales = sales_price*annual_production

Aka - the commodity sell price should provided

in non-escalated dollars (I think this means real - if I'm interpreting Jared's helpful doc page properly)

in the same cost year as set in plant_config["finance_parameters"]["cost_adjustment_parameters"]["target_dollar_year"] (the commodity_sell_price does not go through AdjustedCapexOpexComp)

I would be happy to update the docs! @jaredthomas68 - can you confirm that the commodity_sell_price should be input in real dollars?

Can yall (@vijay092 and @jaredthomas68) tell me whether this is True or false:

if using an inflation rate of 0 in ProFAST, then the commodity sell price should be provided in nominal dollars?

if using a nonzero inflation rate in ProFAST, then the commodity sell price should be provided in real dollars since ProFAST will escalate the price?

Can y'all look at the test h2integrate/finances/test/test_profast_npv.py::test_profast_npv_with_inflation and let me know what y'all think?

vijay092 · 2026-07-08T18:49:51Z

    def setup(self):
        """Set up inputs for the NPV calculation.

        Retrieves the commodity sell price and its units from the plant configuration


Could we add here that commodity_sell_price could be an array?

vijay092 · 2026-07-08T18:51:42Z


        super().setup()

+        if isinstance(self.commodity_sell_price, float | int):


Nit: Good to check for an edge case when use user passes in [x] (a single element list).

Clarifying question: do you think it should be OK if a user passes in a single-element list? So - if it's a list of length 1 then don't throw an error?

I think it is ok to pass a list of length one. We can use the first element!

understood! I think I'm torn on this. I see how it can be more user-friendly but I think that it maybe complicates the "checking" logic in a way that may make the "checking" code slightly harder to understand. If other folks have opinions on this - would love some additional thoughts (@johnjasa, @jaredthomas68)

…ar_sell_price

made it so profast npv can take in multi-year sell price costs

3c03c3b

elenya-grant added the ready for review This PR is ready for input from folks label Jul 6, 2026

Merge remote-tracking branch 'h2i_upstream/develop' into npv/multi_ye…

c6c922f

…ar_sell_price

elenya-grant requested review from jaredthomas68, johnjasa and vijay092 July 7, 2026 16:03

vijay092 reviewed Jul 8, 2026

View reviewed changes

elenya-grant added 3 commits July 8, 2026 13:57

updated error handling for misisng sell price value

62ae478

added subtest for inflation test

cf985dc

Merge remote-tracking branch 'h2i_upstream/develop' into npv/multi_ye…

1b57833

…ar_sell_price


		super().setup()

		if isinstance(self.commodity_sell_price, float \| int):

Uh oh!

Conversation

elenya-grant commented Jul 6, 2026

Multi-year sell-price in ProFAST NPV

Section 1: Type of Contribution

Section 2: Draft PR Checklist

TODO:

Type of Reviewer Feedback Requested (on Draft PR)

Section 3: General PR Checklist

Section 4: Related Issues

Section 5: Impacted Areas of the Software

Section 5.1: New Files

Section 5.2: Modified Files

Section 6: Additional Supporting Information

Section 7: Test Results, if applicable

Uh oh!

vijay092 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elenya-grant Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elenya-grant Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

elenya-grant Jul 8, 2026 •

edited

Loading

elenya-grant Jul 8, 2026 •

edited

Loading