Add per-year pricing support for Grid and Feedstock cost models#700
Add per-year pricing support for Grid and Feedstock cost models#700johnjasa wants to merge 9 commits intoNatLabRockies:developfrom
Conversation
elenya-grant
left a comment
There was a problem hiding this comment.
I gave some minor suggestions but nothing that was blocking! I did have a question on whether the grid cost model is properly set-up to handle non-yearly simulations that I'd like your take on!
I think you added a lot of great tests for the grid and feedstock model! I'm curious why didn't you add any tests to h2integrate/core/test/test_utilities.py that specifically only test the determine_price_mode function?
| varopex += np.sum((self.dt / 3600) * electricity_out * buy_price) | ||
| if self._buy_price_mode == "per_year": | ||
| # Per-year price: total energy * price per year | ||
| total_energy_bought = np.sum(electricity_out) * (self.dt / 3600) |
There was a problem hiding this comment.
Nonblocking: What if instead of always using electricity_sold and electricity_out as inputs to the cost model - what if the inputs were based on the _buy_price_mode? This would perhaps require adding more outputs to the grid performance model. But - it could be something like:
if self._buy_price_mode == "per_year"
self.add_input("annual_electricity_out", val=0.0, shape=plant_life, units="kW*h/yr")
else:
self.add_input("electricity_out", val=0.0, shape=n_timesteps, units="kW")
if self._sell_price_mode == "per_year"
self.add_input("annual_electricity_sold", val=0.0, shape=plant_life, units="kW*h/yr") # this would have to be added as an output to the grid performance model
else:
self.add_input("electricity_sold", val=0.0, shape=n_timesteps, units="kW")There was a problem hiding this comment.
Thanks for drafting up how this could look! It's slightly unclear to me what the value-add would be here. The amount of electricity bought or sold doesn't vary year-to-year, so the annual_electricity_out and annual_electricity_sold arrays would have the same value repeated across the plant_life. Is your suggestion more to reduce the size of the arrays used in the calculations? I'm probably missing something!
There was a problem hiding this comment.
The annual_electricity_out is flexible to varying dt and different simulation lengths (output units are kW*h/yr). electricity_out and electricity_sold are output in rate units (kW) and not adjusted based on time-step or simulation length. My suggestion is so that the cost model doesn't have to do the adjustments for dt and n_timesteps when it could be done by the performance model.
There was a problem hiding this comment.
I just implemented this! We're on a call. Could you please take a look and tell me if it's close to what you want?
| total_energy = np.sum(electricity_out) * (dt / 3600) | ||
| expected_varopex = total_energy * np.array(buy_prices) | ||
|
|
||
| varopex = prob.get_val("grid.VarOpEx", units="USD/year") |
There was a problem hiding this comment.
I think I'm concerned because the simulation is only 24 timesteps but I'm not sure if the units make sense for the VarOpEx. For an hourly simulation of 24 timesteps. The total energy is units of kW*d (kW*h*24), then multiplied by buy price cost of USD/kW*h does not result in units of USD/year. Right? I don't think the grid cost model (and perhaps the grid performance model too) is flexible to shorter simulation times.
The VarOpEx costs in the ProFAST finance models are divided by the total production of the commodity to get the variable O&M cost value in units of USD/commodity_amount_units (commodity_amount_units is for the commodity defined for the finance model). I have to think on this a bit more though...
There was a problem hiding this comment.
You're absolutely right about the grid components not being generalized for non-yearly production. I've updated the tests here. This would require a bigger set of changes beyond just this PR and should be done as part of the non-yearly simulation change work.
| return config["shared_parameters"] | ||
|
|
||
|
|
||
| def determine_price_mode(price, n_timesteps, plant_life, price_name="price"): |
There was a problem hiding this comment.
Nice way to put this as a shared utility function - I think this will be helpful for other models in the future!
There was a problem hiding this comment.
Looking great, John! Just a couple of thoughts:
i) A dictionary format for the sell_price array (e.g. explicitly tagging it as a yearly or daily profile) would be nice. You shouldn't have to infer the type from the array size.
ii) Maybe for later: Is there a straightforward way to pull in price forecasts automatically, to reduce the burden on the user (asking for a friend ;))
| | `fixed_interconnection_cost` | scalar | $ | One-time fixed cost regardless of size. | | ||
| | `electricity_out` | array[n_timesteps] | kW | Electricity flowing out of grid (buying from grid). | | ||
| | `electricity_buy_price` | scalar/array[n_timesteps] | $/kWh | Price to buy electricity from grid (optional, time-varying supported). | | ||
| | `electricity_buy_price` | scalar/array[n_timesteps]/array[plant_life] | $/kWh | Price to buy electricity from grid (optional; supports scalar, per-timestep, or per-year). | |
There was a problem hiding this comment.
Could this be expressed as a dictionary instead? e.g. {"dimension": plant_life, "val": [...]}. Would remove any ambiguity for the user.
There was a problem hiding this comment.
You're right and I like your suggestion! I've changed it so the buy price and sell price modes are set by the user in the config file to make it clear.
|
|
||
| else: | ||
| buy_price_shape = 1 | ||
| self._buy_price_mode, buy_price_shape = determine_price_mode( |
There was a problem hiding this comment.
If you had the dictionary, you probably don't need this function.
There was a problem hiding this comment.
I've removed this in favor of your approach -- thanks for the simplification!
Great suggestions, thanks Sanjana! I've made the change you suggested so users explicitly say what the buy or sell price is. Then I've made an issue for future work to pull the price forecasts: #716. I will say we haven't historically prioritized grid-integrated systems, so as you continue to think up good features to add in for your work, please bring it up or create an issue! |
vijay092
left a comment
There was a problem hiding this comment.
Thanks for incorporating my suggestions, John! Looks great!
Add per-year pricing support for Grid and Feedstock cost models
Adds the ability to specify
electricity_buy_price,electricity_sell_price(Grid), andprice(Feedstock) as arrays of lengthplant_lifefor per-year pricing. Previously only scalar or per-timestep (lengthn_timesteps) arrays were supported.A shared
determine_price_mode()utility function handles validation and mode detection for reuse across models. Whenn_timesteps == plant_life, the per-year interpretation takes priority with aUserWarning.Section 1: Type of Contribution
Section 2: Draft PR Checklist
TODO:
Type of Reviewer Feedback Requested (on Draft PR)
Structural feedback:
Implementation feedback:
Other feedback:
Section 3: General PR Checklist
docs/files are up-to-date, or added when necessaryCHANGELOG.md"A complete thought. [PR XYZ]((https://github.com/NatLabRockies/H2Integrate/pull/XYZ)", where
XYZshould be replaced with the actual number.Section 4: Related Issues
Resolves #660
Section 5: Impacted Areas of the Software
Section 5.1: New Files
None.
Section 5.2: Modified Files
h2integrate/core/utilities.pydetermine_price_mode: New utility function to classify price arrays as scalar, per-timestep, or per-year.h2integrate/converters/grid/grid.pyGridCostModel.setup: Refactored to usedetermine_price_modefor buy/sell price validation.GridCostModel.compute: Updated VarOpEx calculation to produce per-year array for per-year pricing.h2integrate/core/feedstocks.pyFeedstockCostModel.setup: Refactored to usedetermine_price_modefor price validation.FeedstockCostModel.compute: Updated VarOpEx calculation to produce per-year array for per-year pricing.docs/technology_models/grid.mddocs/technology_models/feedstocks.mdh2integrate/converters/grid/test/test_grid.pyh2integrate/core/test/test_feedstocks.pySection 6: Additional Supporting Information
Section 7: Test Results, if applicable