Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding waiting point based rebalancing strategy #74

Merged
merged 7 commits into from
May 16, 2024
Merged

Conversation

luchengqi7
Copy link
Contributor

@luchengqi7 luchengqi7 commented May 10, 2024

By specifying the path to the waiting points file (tsv/csv file) in the run argument (--waiting-points /path/to/waiting/point/file.tsv), the waiting point based rebalancing algorithm will be automatically enabled for av. Note that the rebalancing params for av in the config file needs to be turned on and set to "CustomRebalancingStrategyParams". The preferred interval of waiting point based rebalancing strategy is 1 seconds.

Format of the waiting points file:
link_id (\t) capacity
123456#3 (\t) 1
222222#0 (\t) 3

Next, I will add the rebalancing strategy to the matsim-lib, and then it will be easier to setup. But this may take some time (as we need some internal reviews before it can be finally published). Before that, we can use the current implementation inside this repository.

@luchengqi7 luchengqi7 requested review from rakow and tschlenther May 10, 2024 08:22
Copy link
Contributor

@tschlenther tschlenther left a comment

Choose a reason for hiding this comment

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

I would suggest to move the rebalancing package one level up, i.e. not to have it in the run package.

Further, I like the idea suggested by your comment, that the default waiting point locations are read from the drt vehicles input file. In this case, capacities could be summed up per starting link.
I am not sure, but I think this is not implemented, yet? As we are looking in multiple fleet sizes for multiple areas, this would be quite convenient, at least for us in Kelheim.
See also the code comments.

}
}
}
Preconditions.checkArgument(nearestWaitingPoint != null, "Not able to find nearest waiting point! Please make sure the sum capacity" +
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe we should clarify to the user in form of a JvaDoc for the RebalancingStrategy, that enough capacity needs to be provided. At least when we move the code into matsim-libs

Copy link
Contributor

Choose a reason for hiding this comment

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

Aternatively to aborting, one could argue to leave the vehicle where it is, or to speculatively send it to the nearest waiting point, hoping that that is cleared in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will do the refactoring.

I will add the using start location as the depot. That is indeed more handy for us.


@Parameter("waiting points of the vehicle")
@Comment("The path to the waiting point file (csv/tsv) can be specified here. title row of the file: link_id capacity" +
"If unspecified (i.e., empty string by default), starting points of the fleet will be used as the waiting points")
Copy link
Contributor

Choose a reason for hiding this comment

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

how does this work. The code in RunKelheimScenario reads like this rebalancing strategy would not be installed, if no waitin points file path wass provided. WaitingPointsBasedRebalancingStrategy.initialize() also seems to assume to always read in a .tsv file.
Am i missing something?
Actually, you are right: For Kelheim, it would be convenient to use the start locations, as we have spreaded them throughout the corresponding service areas with some thoughts (they represent the waiting points for the realistic fleet sizes). Also, we would have to come up with one file per fleet size, because the current version throws an error if the total capacity is insufficent. That is not a major problem, of course.

@Override
public List<Relocation> calcRelocations(Stream<? extends DvrpVehicle> rebalancableVehicles, double time) {
List<Relocation> relocations = new ArrayList<>();
List<? extends DvrpVehicle> idleVehicles = rebalancableVehicles.filter(v -> v.getServiceEndTime() > time + params.minServiceTime).toList();
Copy link
Contributor

Choose a reason for hiding this comment

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

is the order of the list random? Should it be? Maybe not for our small-fleet-size cases in Kelheim, but for other, large-fleet scenarios it might matter.
Because if for example the list is ordered by the vehicle Ids, it would always be the same vehicle(s) that have to clear a capacity-exceeded waiting point or that get to fill up the closes waiting point etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I can add a shuffle operation to avoid this problem.

But it needs to be pointed out that it doesn't guarantee the results will get better. It's just 2 different heuristic solutions, and sometimes one is better, and sometimes the other is better.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's true!

/**
* @author Chengqi Lu
*/
public class WaitingPointsBasedRebalancingStrategy implements RebalancingStrategy {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this class should not be public

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why not? Maybe not in the kelheim scenario. But probably in the matsim-lib after it is moved there, just like the other rebalancing strategies. 😀

Copy link
Contributor

Choose a reason for hiding this comment

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

Because the WaitingPointsBasedRebalancingModule is what is the outward-facing API. According to KN, we want as less public footprint as possible, in order to be able to change internal API.

@tschlenther tschlenther self-requested a review May 16, 2024 14:16
tschlenther and others added 2 commits May 16, 2024 16:16
Change visibility to package-private (i.e., without public statement in front)
Copy link

Quality Gate Passed Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@luchengqi7 luchengqi7 merged commit 70d89ec into master May 16, 2024
4 of 5 checks passed
@tschlenther tschlenther deleted the rebalancing branch August 8, 2024 09:37
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.

2 participants