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

issue with predefined facilities results? #237

Closed
jGaboardi opened this issue May 31, 2022 · 7 comments
Closed

issue with predefined facilities results? #237

jGaboardi opened this issue May 31, 2022 · 7 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@jGaboardi
Copy link
Member

@gegen07 I may have found an issue with the result of defining facilities for the LSCP, though I may have misunderstood something. Check out the gist here that is bit altered from our LSCP example notebook (cells 2, 14, 22, 23). Basically, the results in cells 22 and 23 seem show that not all clients are being covered by facilities. Do you have time to look into this?

@jGaboardi jGaboardi added bug Something isn't working question Further information is requested labels May 31, 2022
@gegen07
Copy link
Member

gegen07 commented Jun 1, 2022

Hmm... I just replicate your gist and seems that the problem with predefined locations + geopandas is infeasible. Possibly because of the strict max_coverage=3. When we use geopandas we have to calculate the distances using cdist with euclidean metric as default and then some distances are different compared to Dijkstra calculated by spaghetti. So it works for from_cost_matrix and doesn't work for from_geodataframe.

I tried with max_coverage=5 and it seems that is right.

@qszhao
Copy link

qszhao commented Jun 1, 2022

Hi @gegen07 if this is an LSCP problem, then we need to cover all the demands. In this case, the algorithm should try to add additional facilities to cover the uncovered demands. It has nothing to do with the max_coverage in this case??

@jGaboardi
Copy link
Member Author

Hi @gegen07 if this is an LSCP problem, then we need to cover all the demands. In this case, the algorithm should try to add additional facilities to cover the uncovered demands. It has nothing to do with the max_coverage in this case??

Here max_coverage means the service radius threshold $S$.

@jGaboardi
Copy link
Member Author

OK, figured it out. In cell 2 I set the solver output to True to see the CBC logs. The models are infeasible with the service radius I had set (MAX_COVERAGE = 3).

We should probably be checking solution status and raising an exception if the model is infeasible.

@qszhao
Copy link

qszhao commented Jun 1, 2022

That's a very good idea to give the red flag! @jGaboardi

@gegen07
Copy link
Member

gegen07 commented Jun 1, 2022

Maybe we also should change the name max_coverage to service_radius to avoid misunderstanding.

@gegen07
Copy link
Member

gegen07 commented Jun 2, 2022

xref #239

@gegen07 gegen07 closed this as completed Jun 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants