Rename investmentset marketset

[dc2917]

Description

This PR:

1. Moves InvestmentSet and associated functions out of the investment module into a new market module (with some (un)educated guesses at which functions should be moved, and which functions should remain but be made public))
2. Renames InvestmentSet to MarketSet
3. Updates graph::investment to reflect these changes (also with some guesswork at which terminology should be updated)

Fixes #1337

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
• Update release notes for the latest release if this PR adds a new feature or fixes a bug
  present in the previous release

Further checks

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

[codecov]

Codecov Report

❌ Patch coverage is 94.94048% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.68%. Comparing base (76db0a2) to head (eb225bd).
⚠️ Report is 39 commits behind head on main.

Files with missing lines	Patch %	Lines
src/simulation/market.rs	94.73%	13 Missing and 3 partials ⚠️
src/simulation/investment.rs	88.88%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1371      +/-   ##
==========================================
- Coverage   89.80%   89.68%   -0.12%     
==========================================
  Files          58       59       +1     
  Lines        8524     8406     -118     
  Branches     8524     8406     -118     
==========================================
- Hits         7655     7539     -116     
+ Misses        556      550       -6     
- Partials      313      317       +4     

☔ View full report in Codecov by Harness.
📢 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]

On board with the name change. I think some of the stuff moved over from simulation::investment to simulation::market should probably still belong in simulation::investment, particularly the select_assets functions, and possibly even get_demand_portion_for_market and get_responsible_agents. That get's a bit messy since select_assets is a method of MarketSet. I think you can define methods away from where the struct lives, but maybe that's messy, and maybe select_assets would be better as a standalone function anyway?

@alexdewar what do you think?

[alexdewar]

Looking good! I've suggested a few tweaks

[alexdewar]

This function and get_asset_options are only used in market.rs now, so they could be left private and moved there.

select_best_assets is also only used in market.rs, but given that that's the heart of the investment-related code, I feel like it fits better in investment.rs.

Do you agree @tsmbland?

[tsmbland]

I do think all the core investment-related code should be in investment.rs, and leave market.rs for generic market-related stuff like the iter and display implementations

[dc2917]

I've given things a bit of a shuffle, trying to accommodate both of your suggestions 😅

I've tried to use a bit of logic based on how/where the functions are used and how they're described in their docstrings (and names), but I obviously have no personal feelings on the matter so am happy to move things as you see fit.

[alexdewar]

These identifiers can be turned into links for the docs:

Suggested change

	/// Assets are selected for a single market using `select_assets_for_single_market`
	Single((CommodityID, RegionID)),
	/// Assets are selected for a group of markets which forms a cycle.
	/// Experimental: handled by `select_assets_for_cycle` and guarded by the broken options
	/// parameter.
	Cycle(Vec<(CommodityID, RegionID)>),
	/// Assets are selected for a layer of independent `MarketSet`s
	/// Assets are selected for a single market using [`select_assets_for_single_market`]
	Single((CommodityID, RegionID)),
	/// Assets are selected for a group of markets which forms a cycle.
	/// Experimental: handled by [`select_assets_for_cycle`]. May not work in every case.
	Cycle(Vec<(CommodityID, RegionID)>),
	/// Assets are selected for a layer of independent [`MarketSet`]s

Edit: Also changed the text for Cycle as it's no longer a hidden option.

[dc2917]

@alexdewar there may be a reason why these weren't linked before:

Generating API documentation
 Documenting muse2 v2.1.0 (/home/runner/work/MUSE2/MUSE2)
error: public documentation for `Single` links to private item `select_assets_for_single_market`
Error:   --> src/simulation/market.rs:26:57
   |
26 |     /// Assets are selected for a single market using [`select_assets_for_single_market`]
   |                                                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this item is private
   |
   = note: this link resolves only because you passed `--document-private-items`, but will break without
   = note: `-D rustdoc::private-intra-doc-links` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(rustdoc::private_intra_doc_links)]`

error: public documentation for `Cycle` links to private item `select_assets_for_cycle`
Error:   --> src/simulation/market.rs:29:36
   |
29 |     /// Experimental: handled by [`select_assets_for_cycle`]. May not work in every case.
   |                                    ^^^^^^^^^^^^^^^^^^^^^^^ this item is private
   |
   = note: this link resolves only because you passed `--document-private-items`, but will break without

[alexdewar]

On board with the name change. I think some of the stuff moved over from simulation::investment to simulation::market should probably still belong in simulation::investment, particularly the select_assets functions, and possibly even get_demand_portion_for_market and get_responsible_agents. That get's a bit messy since select_assets is a method of MarketSet. I think you can define methods away from where the struct lives, but maybe that's messy, and maybe select_assets would be better as a standalone function anyway?

@alexdewar what do you think?

Ha, I just messaged you suggesting kind of the opposite (moving more things to market.rs) and asking what you think. I can see it both ways, but I feel like getting the set of assets we might invest in isn't "core" investment code (there's no investment) and given that there's quite a lot of this code, maybe it would be better somewhere else. investment.rs is quite long, so it would be nice to trim it down a bit.