Add data structure for asset pool #388
Conversation
There is currently a problem in that the last milestone year is after all of the assets have been decommissioned, meaning that there will be no assets available for the last milestone year, which seems pretty nonsensical (albeit theoretically possible) and it causes the optimiser to barf. Let's fix this by changing the last milestone year.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #388 +/- ##
==========================================
+ Coverage 95.33% 95.42% +0.09%
==========================================
Files 31 31
Lines 3882 4004 +122
Branches 3882 4004 +122
==========================================
+ Hits 3701 3821 +120
- Misses 94 96 +2
Partials 87 87 ☔ View full report in Codecov by Sentry. |
We need to do this otherwise the problem is infeasible: there is not sufficient supply to meet service demands for RSHEAT in the final milestone year.
There was a problem hiding this comment.
Approving because the code makes sense to me. I just have a couple of question which, to be honest, may be born out of me not understanding the use scenario of assets becoming active/inactive;
When an asset becomes inactive, i.e. setting the AssetID to INACTIVE, do we not lose the ability to lookup that Asset by its unique identifier (AssetID) in the future? Or perhaps it doesn't matter because it gets dropped from the AssetPool anyway?
Just checking I understand the logic really.
| pub fn commission_new(&mut self, year: u32) { | ||
| assert!( | ||
| year >= self.current_year, | ||
| "Assets have already been commissioned for year {year}" | ||
| ); | ||
| self.current_year = year; | ||
| } | ||
|
|
||
| /// Decommission old assets for the specified milestone year | ||
| pub fn decomission_old(&mut self, year: u32) { | ||
| assert!( | ||
| year >= self.current_year, | ||
| "Cannot decommission assets in the past (current year: {})", | ||
| self.current_year | ||
| ); | ||
| self.assets.retain(|asset| asset.decommission_year() > year); | ||
| } |
There was a problem hiding this comment.
Just so I understand; So all commission_new() is doing is updating the current_year for the AssetPool. decommission_old is doing the important bit by making sure that only assets whose decommission year is in the future is kept in AssetPool.
Do we not want to set the id field of the Asset to AssetID::INVALID here also? Or is that not the intended use of that value? Does "decommissioned" have a different meaning to "inactive"?
There was a problem hiding this comment.
Just so I understand; So all
commission_new()is doing is updating thecurrent_yearfor theAssetPool.decommission_oldis doing the important bit by making sure that only assets whose decommission year is in the future is kept inAssetPool.
Yep, exactly. Because the assets are sorted by commission year, to get the active assets we can then just iterate over the Vec until we find a commission year > current_year.
Decommissioned assets are just deleted from the Vec for now, though we could potentially just mark them as inactive in some way instead (maybe replace them with None?), which would have the advantage that we wouldn't have to move memory around every time we decommission things. But I just went for deleting them for now because that seemed easier.
Do we not want to set the
idfield of theAssettoAssetID::INVALIDhere also? Or is that not the intended use of that value? Does "decommissioned" have a different meaning to "inactive"?
I've noticed that the doc comments are a bit confusing here. AssetID::INVALID is just the ID that every Asset is given when it's created. When we move the assets into the AssetPool, then at that point they're all given a unique ID. The reason for doing it at this point is so that we can sort the assets by commission year and then give them IDs in that same order, which will make it faster to look up assets by ID. We could also just sort the assets when we load them in, but it seemed a bit fragile for AssetPool to rely on this (we might refactor the input code and subtly break it).
Description
Currently
AssetPoolis just a type alias for aVec<Asset>and in order to get the currently commissioned assets we just filter thisVec. However, going forward we will want to be able to modify this data structure as we go along: assets are commissioned and decommissioned based on the milestone year and agents can also select from among them in the agent investment step. So I think it makes sense to have a dedicated struct for this with some methods.It has the following functionality:
After I implemented the decommissioning step I noticed I was getting error messages from the optimiser about it being an empty problem: this is because all of the assets have been commissioned by the time of the last milestone year. To fix this I just moved the milestone year, but at some point we probably want to figure out how to handle this scenario cleanly.
Closes #313. Closes #335.
Type of change
Key checklist
$ cargo test$ cargo docFurther checks