Skip to content

Fix: Don't calculate prices directly from levies#889

Merged
tsmbland merged 1 commit into
mainfrom
fix-levies
Oct 7, 2025
Merged

Fix: Don't calculate prices directly from levies#889
tsmbland merged 1 commit into
mainfrom
fix-levies

Conversation

@alexdewar
Copy link
Copy Markdown
Member

Description

In general in MUSE2, commodities get their prices as a byproduct of the dispatch optimisation; specifically we use dual values for the commodity balance constraints (aka shadow prices). The commodity balance constraints are only applied to SED and SVD commodities, however, so OTH-type commodities will not get a price this way: in the example models, the only commodity of this type is CO2 (which makes sense -- no one's buying or selling it). There is a second way that we calculate prices: if a commodity has a levy, we use that as the price (or the shadow price, if that's greater). This means that in the examples, CO2 is always given a price equal to the levy.

This scheme breaks things when we calculate (what we formerly called) reduced costs, needed as a first step to calculate LCOX. The reason is because we calculate LCOX by subtracting an asset's costs (derived from variable operating cost, flow costs and levies) from its flow revenues (derived from prices). By giving CO2 a price, it means that it is erroneously included in the equation twice. On reflection, I don't think it makes sense to give CO2 a "price" at all. Bear in mind that the implication of giving it a price (at least in this specific context) is that someone is paying for the CO2 in the same way they pay for other output commodities, which isn't right! In fact, including a price for CO2 here cancels out the effect of including the CO2 tax when we calculate asset costs. Sure, we could give CO2 a negative price instead, but then the effect would be to double the cost of CO2, which is also wrong.

I think the solution is just not to use the values of levies when assigning commodities prices -- do that and the double-counting problem goes away. I believe that there is some overlap between the concepts of levies and prices in MUSE1, but I don't think it makes sense in the context of MUSE2.

I tried replicating the numbers that @ahawkes manually calculated for LCOX (see #876 for data) for the RGASBR existing asset (as this should now be being calculated correctly) and now the (wrongly named) reduced costs match up, however we ultimately get a different result for the cost index because the activities are different. In any case, I think we should merge this PR (assuming everyone's happy with it), but does anyone have any ideas about why we might be seeing this discrepancy before I dig into it some more?

Fixes #877.

Type of change

  • Bug fix (non-breaking change to fix an issue)
  • New feature (non-breaking change to add functionality)
  • Refactoring (non-breaking, non-functional change to improve maintainability)
  • Optimization (non-breaking change to speed up the code)
  • Breaking change (whatever its nature)
  • Documentation (improve or add documentation)

Key checklist

  • All tests pass: $ cargo test
  • The documentation builds and looks OK: $ cargo doc

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 6, 2025

Codecov Report

❌ Patch coverage is 33.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.37%. Comparing base (0946e61) to head (feda065).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/simulation/prices.rs 33.33% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #889      +/-   ##
==========================================
- Coverage   85.38%   85.37%   -0.02%     
==========================================
  Files          50       50              
  Lines        5337     5319      -18     
  Branches     5337     5319      -18     
==========================================
- Hits         4557     4541      -16     
+ Misses        560      559       -1     
+ Partials      220      219       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Removes levy-based price assignment to prevent double-counting (especially for CO2/OTH commodities) and updates expected outputs accordingly. Key changes:

  • Stop incorporating levies when computing commodity prices; use only duals (and scarcity adjustment where configured).
  • Remove the CommodityPrices::with_levies helper and adjust pricing strategy logic.
  • Refresh test fixtures (prices, flows, duals, reduced costs, assets) to match the new pricing behavior.

Reviewed Changes

Copilot reviewed 13 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/simulation/prices.rs Remove levy-based price assignment; delete with_levies; keep scarcity-adjusted logic consistent.
tests/data/simple/commodity_prices.csv Remove CO2 price rows and update electricity prices under scarcity; reflects new pricing logic.
tests/data/simple/debug_solver.csv Update solver objective traces due to changed prices/flows.
tests/data/simple/debug_reduced_costs.csv Update reduced cost outputs to remove prior levy-related effects.
tests/data/simple/debug_commodity_balance_duals.csv Update dual values consistent with changed prices and iteration paths.
tests/data/simple/debug_appraisal_results.csv Update appraisal metrics impacted by pricing/removal of levies.
tests/data/simple/commodity_flows.csv Update flows to reflect new dispatch and pricing.
tests/data/simple/assets.csv Adjust assets timeline to reflect new dispatch under revised prices.
tests/data/two_regions/commodity_prices.csv Remove CO2f price lines; keep only commodities priced via duals.
tests/data/muse1_default/commodity_prices.csv Remove CO2f levy-based prices.
tests/data/missing_commodity/commodity_prices.csv Remove CO2EMT levy-based prices.
tests/data/missing_commodity/commodity_flows.csv Update flows affected by the pricing change.
tests/data/missing_commodity/assets.csv Adjust asset data per new dispatch solution.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread src/simulation/prices.rs
@alexdewar
Copy link
Copy Markdown
Member Author

Btw on the issue page, @ahawkes has said:

Yes, works for CO2 but not for your coal example (which has a shadow price). If there's a shadow price for the commodity, we do not include the levy via the operating cost element.

Just to note that I haven't done that on this PR (yet), though I could do. What do you think @tsmbland?

@ahawkes
Copy link
Copy Markdown
Contributor

ahawkes commented Oct 6, 2025

That's odd re changed activity - I don't see how it would be possible. Looks like RELCHP also not giving the same cost index that I got? I see in the results the correct value for unmet demand (I see 23.619 for RGASBR and 706.183 for RELCHP), so how could activity have changed?

Copy link
Copy Markdown
Collaborator

@tsmbland tsmbland left a comment

Choose a reason for hiding this comment

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

I think this is a good first step, although as mentioned this doesn't fix everything

I don't see the problem RE activity. Activity is unchanged in the first year. After that investment decisions change (presumably because the CO2 levy is now being properly accounted for), so no surprise that there are changes to activity. Am I missing something?

@ahawkes
Copy link
Copy Markdown
Contributor

ahawkes commented Oct 6, 2025 via email

@tsmbland
Copy link
Copy Markdown
Collaborator

tsmbland commented Oct 7, 2025

@alexdewar Merging this for you because I want to build off it

@tsmbland tsmbland merged commit 1cacfaf into main Oct 7, 2025
8 checks passed
@tsmbland tsmbland deleted the fix-levies branch October 7, 2025 08:13
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.

Commodities with levies double-counted in reduced cost calculation

4 participants