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

Facility capacity for LSCP #273

Merged
merged 35 commits into from
Dec 14, 2022
Merged

Facility capacity for LSCP #273

merged 35 commits into from
Dec 14, 2022

Conversation

erinrolson
Copy link
Contributor

@erinrolson erinrolson commented Sep 8, 2022

Implements facility capacity constraint for solving LSCP for GSoC '22

@codecov
Copy link

codecov bot commented Sep 8, 2022

Codecov Report

Merging #273 (0771adc) into main (ecfe2cc) will decrease coverage by 0.7%.
The diff coverage is 24.2%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main    #273     +/-   ##
=======================================
- Coverage   74.0%   73.3%   -0.7%     
=======================================
  Files         23      23             
  Lines       2421    2452     +31     
  Branches     463     475     +12     
=======================================
+ Hits        1791    1797      +6     
- Misses       572     591     +19     
- Partials      58      64      +6     
Impacted Files Coverage Δ
spopt/locate/coverage.py 93.6% <22.2%> (-6.4%) ⬇️
spopt/locate/base.py 88.5% <26.7%> (-6.5%) ⬇️

Comment on lines 11 to 37
"# Location Set Covering Problem (LSCP)\n",
"\n",
"*Authors:* [Erin Olson](https://github.com/erinrolson),[Germano Barcelos](https://github.com/gegen07), [James Gaboardi](https://github.com/jGaboardi), [Levi J. Wolf](https://github.com/ljwolf), [Qunshan Zhao](https://github.com/qszhao)\n",
"\n",
"Location Set Covering is a problem realized by Toregas, et al. (1971). He figured out that emergency services must have placed according to a response time, since, there is a allowable maximum service time when it's discussed how handle an emergency activity. Therefore he proprosed a model named LSCP that:\n",
"\n",
"_Minimize the number of facilities needed and locate them so that every demand area is covered within a predefined maximal service distance or time._ Church L., Murray, A. (2018)\n",
"\n",
"**LSCP can be written as:**\n",
"\n",
"$\\begin{array} \\displaystyle \\textbf{Minimize} & \\sum_{j=1}^{n}{x_j} && (1) \\\\\n",
"\\displaystyle \\textbf{Subject to:} & \\sum_{j\\in N_i}{x_j} \\geq 1 & \\forall i & (2) \\\\\n",
" & x_j \\in {0,1} & \\forall j & (3) \\\\ \\end{array}$\n",
" \n",
"$\\begin{array} \\displaystyle \\textbf{Where:}\\\\ & & \\displaystyle i & \\small = & \\textrm{index referencing nodes of the network as demand} \\\\\n",
"& & j & \\small = & \\textrm{index referencing nodes of the network as potential facility sites} \\\\\n",
"& & S & \\small = & \\textrm{maximal acceptable service distance or time standard} \\\\\n",
"& & d_{ij} & \\small = & \\textrm{shortest distance or travel time between nodes} \\quad i \\quad \\textrm{and} \\quad j \\\\\n",
"& & N_i & \\small = & \\{j | d_{ij} < S\\} \\\\\n",
"& & x_j & \\small = & \\begin{cases} \n",
" 1, \\text{if a facility is located at node} \\quad j\\\\\n",
" 0, \\text{otherwise} \\\\\n",
" \\end{cases} \\end{array}$\n",
" \n",
"_This excerpt above was quoted from Church L., Murray, A. (2018)_\n",
"\n",
"This tutorial solves LSCP using `spopt.locate.coverage.LSCP` instance that depends on a array 2D representing the costs between facilities candidate sites and demand points. For that it uses a lattice 10x10 with simulated points to calculate the costs."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update intro with new description and reference/display new capacity formulation

@jGaboardi
Copy link
Member

@qszhao

@jGaboardi
Copy link
Member

@ljwolf conflicts were minor and now are resolved.

Copy link
Member

@ljwolf ljwolf left a comment

Choose a reason for hiding this comment

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

I have one suggestion on the error warning, and would also suggest a bit more narrative in the example notebook. But, I'll need to commit that separately from this review.

spopt/locate/base.py Outdated Show resolved Hide resolved
@ljwolf
Copy link
Member

ljwolf commented Dec 14, 2022

OK, I've updated the notebook wording. I will merge once the tests pass.

@jGaboardi
Copy link
Member

Just realized there are no tests here. But I suppose we can add later. The functionality is demoed in a basic way in the notebook and that can be translated to a simple test.

@ljwolf
Copy link
Member

ljwolf commented Dec 14, 2022

Hmm, I missed that for sure. I think that the notebook serves as a user test, and the existing option passing is tested already. What's not tested is the more finicky stuff, like argument typing and things... I think it's OK here because the functionality is completely checked by the notebook, but I wouldn't want this to justify future code without tests!

@ljwolf ljwolf merged commit 10535e6 into pysal:main Dec 14, 2022
@jGaboardi
Copy link
Member

I think that the notebook serves as a user test, and the existing option passing is tested already. What's not tested is the more finicky stuff, like argument typing and things... I think it's OK here because the functionality is completely checked by the notebook, but I wouldn't want this to justify future code without tests!

My thoughts exactly. I will look into adding to @erinrolson's excellent contribution by doing a thorough doc review, adding tests, and probably adding some spice to the notebook. Hopefully I can get to this in the next couple weeks so we can include in the coming meta release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants