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

Greedy taset after project #65

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open

Greedy taset after project #65

wants to merge 21 commits into from

Conversation

F-I-D-O
Copy link
Member

@F-I-D-O F-I-D-O commented Feb 19, 2022

No description provided.

// initCount += 100;
// }
transferPoints.add(node);
//createStation(node, initCount, counter++);
Copy link
Member Author

Choose a reason for hiding this comment

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

don't forget to remove all these comments

maxDelayTime = config.ridesharing.maxProlongationInSeconds * 1000;
this.config = config;

// // max distance in meters between vehicle and request for the vehicle to be considered to serve the request
Copy link
Member Author

Choose a reason for hiding this comment

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

Why have you replaced the values from config with some hardcoded ones?

this.requestFactory = requestFactory;

//TODO:
// commented because config is null in test
Copy link
Member Author

Choose a reason for hiding this comment

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

You should make it work both with tests and integrated into the simulation. You can provide config in tests too.

for (int i = 0; i < taxisCount; i++) {
RideSharingOnDemandVehicle taxi = taxis.get(i);
SimulationNode taxiPosition = taxis.get(i).getPosition();
Set<PlanComputationRequest> requestsOnBoardSet = new HashSet<>();
Copy link
Member Author

Choose a reason for hiding this comment

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

Why are you using a set here??

}
List<PlanAction> bestPlan = findItineraryWithMinimumDelay(lst);
return bestPlan;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This function seems to be like IH for one vehicle, but extremely inefficient. I guess later you can achieve huge improvements here.

* @return
*/
// private List<RequestPlan> heuristics(List<RideSharingOnDemandVehicle> taxis, List<PlanComputationRequest> requests) {
private List<RideSharingOnDemandVehicle> heuristics(List<RideSharingOnDemandVehicle> taxis, List<PlanComputationRequest> requests) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Too long method. It should be decomposed almost certainly. I haven't seen any pseudocode remotely close to this in the article :)

Comment on lines 490 to 512
for (int i = 0; i < planOfCar.size(); i++) {
PlanAction action = planOfCar.get(i);
if (action instanceof PlanRequestAction) {
PlanComputationRequest pcq = ((PlanRequestAction) action).getRequest();
previousPosition = action.getPosition();
if (pcq == request) {
index = i;
break;
}
}
}
//find last action
for (int i = planOfCar.size()-1; i > 0; i--) {
PlanAction action = planOfCar.get(i);
if (action instanceof PlanRequestAction) {
PlanComputationRequest pcq = ((PlanRequestAction) action).getRequest();
previousPosition = action.getPosition();
if (pcq == request) {
lastAction = action;
break;
}
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

you should not need a for loop to find first/last action

return bestPlan;
}

private List<PlanAction> getActionsForRequestFromActionsForDriver(List<PlanAction> planActionsVehicle, PlanComputationRequest request, RideSharingOnDemandVehicle vehicle) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Again, seems overly complex for the task

@@ -29,12 +34,16 @@
* @author F.I.D.O.
*/
public class DriverPlan implements Iterable<PlanAction>{
Copy link
Member Author

Choose a reason for hiding this comment

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

This class should not be used for plan creation, it is an output class for the simulation. Create your own data structures for this instead.

Comment on lines +7 to 11
amodsim_data_dir: '/Users/adela/Documents/bakalarka/test_simod/data/'

# experiment specific data (cache, results,...)
amodsim_experiment_dir: "FILL THIS WITH PATH TO EXPERIMENT DIR"
amodsim_experiment_dir: "/Users/adela/Documents/bakalarka/test_simod/"

Copy link
Member Author

Choose a reason for hiding this comment

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

Wrong, put this to your local config!

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.

1 participant