Skip to content

Different approach for using "reduced costs"#891

Merged
tsmbland merged 8 commits into
mainfrom
remove_reduced_costs
Oct 10, 2025
Merged

Different approach for using "reduced costs"#891
tsmbland merged 8 commits into
mainfrom
remove_reduced_costs

Conversation

@tsmbland
Copy link
Copy Markdown
Collaborator

@tsmbland tsmbland commented Oct 7, 2025

Description

This follows on from #888, so look at that one first

We were previously calculating a quantity that we called "reduced costs" from the output of the dispatch optimisation, and passing this the investment algorithm to use during appraisal.

From a linear programming perspective "reduced cost" is "the amount by which an objective function coefficient would have to improve (so increase for maximization problem, decrease for minimization problem) before it would be possible for a corresponding variable to assume a positive value in the optimal solution" (from wikipedia).

Essentially, and as you can see here, we were using this to represent the cost per unit of activity of an asset. This is important for considering potential investments as, depending on the objective, this can be weighed up against potential revenues to assess the profitability of each potential investment.

We were previously calculating "reduced costs" in two different ways for commissioned vs candidate assets:

  • candidate assets: a traditional (I believe) approach for calculating reduced costs using the column duals
  • existing assets: a custom approach using commodity prices and technology parameters

After initially trying this approach, it was discovered that, for our purposes, the first approach doesn't work (or, at least, doesn't give us what we need for investment appraisal). Therefore, we've decided to try using the second approach for all assets. Following on from this, a few extra considerations stood out:

  • The second approach, whilst analogous to reduced costs, does not actually calculate reduced costs in the true linear programming sense of the term, so we should stop calling them as such. Since we should no longer use actual reduced costs anywhere, we should stop using this term throughout the code.
  • There was a separate point that the method for calculating "reduced costs" should vary depending on the objective. This was fixed in Fix: Exclude commodity of interest from reduced costs calculation for LCOX assets #882. However, we were still calculating and returning this value as part of the solution for the dispatch optimisation, and there was the argument that the dispatch optimisation should be agnostic to the investment objectives of the agents, as these are distinct and separate stages of the algorithm.
  • Since method 2 for calculating "reduced costs" doesn't need access to the column duals of the dispatch solution, we can get by with moving this calculation to the investment module, where it only needs to receive commodity prices from the previous dispatch run.

I've tried to address all of these points in this PR. I think this gives the code much clearer flow and greater separation of concerns. "Reduced costs" are no longer calculated, and instead we're passing prices to select_best_assets. What we're now calculating with calculate_activity_coefficient_for_lcox and calculate_activity_coefficient_for_npv is equivalent to what we were previously passing around as "reduced costs" (at least for existing assets, but now we're using the same method for candidate assets as well).

I've tried to brush up the documentation so it's up to date with the current approach. There are still a few small inconsistencies like #383, but I'm not aiming for perfection right now.

I need to think about this more, but there may still be issues with double counting levies for some commodities (#877).

Fixes #879
Fixes #876

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 7, 2025

Codecov Report

❌ Patch coverage is 58.33333% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.69%. Comparing base (44038aa) to head (e19bb1b).

Files with missing lines Patch % Lines
...rc/simulation/investment/appraisal/coefficients.rs 54.83% 14 Missing ⚠️
src/asset.rs 30.00% 7 Missing ⚠️
src/simulation/prices.rs 50.00% 3 Missing ⚠️
src/simulation.rs 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                     Coverage Diff                     @@
##           precompute_coefficients     #891      +/-   ##
===========================================================
+ Coverage                    85.41%   85.69%   +0.27%     
===========================================================
  Files                           50       50              
  Lines                         5335     5186     -149     
  Branches                      5335     5186     -149     
===========================================================
- Hits                          4557     4444     -113     
+ Misses                         559      529      -30     
+ Partials                       219      213       -6     

☔ 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.

@tsmbland
Copy link
Copy Markdown
Collaborator Author

tsmbland commented Oct 7, 2025

No idea why the results are different with mac os... very strange

@tsmbland tsmbland marked this pull request as ready for review October 7, 2025 13:47
@tsmbland tsmbland changed the title No longer pre-computing "reduced costs" Different approach for using "reduced costs" Oct 7, 2025
Copy link
Copy Markdown
Member

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

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

This makes sense to me and looks a lot cleaner than what we had before. Good work!

The different results on macOS ARM are pretty disturbing though... and I don't think we can merge this until we've figured out why it's happening 😞. Are you happy looking into this or would you like me to take a look?

Comment thread docs/model/investment.md Outdated
Comment thread src/asset.rs Outdated
Comment thread src/simulation/prices.rs Outdated
@tsmbland
Copy link
Copy Markdown
Collaborator Author

tsmbland commented Oct 9, 2025

This makes sense to me and looks a lot cleaner than what we had before. Good work!

Thanks!

The different results on macOS ARM are pretty disturbing though... and I don't think we can merge this until we've figured out why it's happening 😞. Are you happy looking into this or would you like me to take a look?

I wouldn't really know where to start to be honest, so if you have any ideas then go ahead!

@alexdewar
Copy link
Copy Markdown
Member

I wouldn't really know where to start to be honest, so if you have any ideas then go ahead!

I'm not sure I have many good ones 😆, but I'll take a look.

@dalonsoa
Copy link
Copy Markdown
Collaborator

dalonsoa commented Oct 9, 2025

Check with Adrian if you need things checked on an ARM macOS at a deeper level that what you can do here.

Base automatically changed from precompute_coefficients to main October 9, 2025 10:17
@alexdewar alexdewar linked an issue Oct 10, 2025 that may be closed by this pull request
Copy link
Copy Markdown
Member

@alexdewar alexdewar left a comment

Choose a reason for hiding this comment

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

Now that the macOS thing is fixed, I think this is good to go.

I've checked with @ahawkes that the LCOX values tally with his spreadsheet, so I've linked this PR to #876 too.

@alexdewar
Copy link
Copy Markdown
Member

...good to go, other than the merge conflict, obvs.

@tsmbland tsmbland enabled auto-merge October 10, 2025 10:25
@tsmbland tsmbland merged commit fe52b59 into main Oct 10, 2025
6 checks passed
@tsmbland tsmbland deleted the remove_reduced_costs branch October 10, 2025 10:26
@alexdewar alexdewar mentioned this pull request Oct 10, 2025
9 tasks
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.

Calculate reduced costs for candidates the same way we do for existing assets Incorrect values for reduced costs

3 participants