-
Notifications
You must be signed in to change notification settings - Fork 54
Add Flow Refueling Location Model (FRLM) #487
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
base: main
Are you sure you want to change the base?
Conversation
fengzixin0617
commented
Jun 3, 2025
- Add FRLM class and example notebook
- With exact and greedy solver options
- Combined capacited FRLM and basic FRLM
- Add FRLM class and example notebook - with exact and greedy solvers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, great first pass! We need some more documentation throughout, and we need to consider this in light of the naming conventions and API style of the other problems.
See the comments across the PR for advice.
"metadata": {}, | ||
"outputs": [], | ||
"source": [ | ||
"G = nx.grid_2d_graph(5, 5)\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'd be great if this used the libpysal.graph.Graph
class instead of networkx
here...
"plt.figure(figsize=(8, 8))\n", | ||
"nx.draw_networkx_nodes(G, pos, node_size=300, node_color='skyblue')\n", | ||
"nx.draw_networkx_edges(G, pos)\n", | ||
"edge_labels = {(u, v): f\"{d['length']:.1f}\" for u, v, d in G.edges(data=True)}\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could use the plotting functionality again in libpysal.graph.Graph
, and would fit better within the library.
] | ||
}, | ||
{ | ||
"cell_type": "code", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We usually start the notebooks about spopt.locate
models with a little bit about the mathematical structure of the MILP being solved. See @gegen07's excellent p-center notebook.
|
||
print("="*60) | ||
|
||
def export_solution(self, filename: str, format: str = 'csv') -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be implemented as separate write_csv()
, write_json()
and write_excel()
methods.
|
||
return utilization | ||
|
||
def print_problem_details(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a summary()
method?