Adding Multiperiod Blending Problem to GDPlib#98
Adding Multiperiod Blending Problem to GDPlib#98arshb11 wants to merge 21 commits intoSECQUOIA:mainfrom
Conversation
bernalde
left a comment
There was a problem hiding this comment.
Welcome to GDPlib, @arshb11! Thank you for this contribution -- adding the multiperiod blending problem with 60 test instances is a valuable addition to the library.
I verified that the model builds correctly across multiple instances and that the GDP transformations (logical_to_linear + bigm) work as expected. The formulation is well-structured with proper disjuncts and logical constraints.
I have a few suggestions below, mostly around aligning with the existing codebase patterns and making sure the new module integrates with the test framework. Nothing major -- happy to iterate on these.
| import gdplib.small_batch | ||
| import gdplib.cstr | ||
| import gdplib.reverse_electrodialysis | ||
| import gdplib.multiperiod_blending |
There was a problem hiding this comment.
Good that this is added here. One thing to be aware of: the test module lists in tests/test_comprehensive_coverage.py (ALL_GDPLIB_MODULES), tests/test_module_imports.py (GDPLIB_MODULES), and tests/test_model_structure.py (GDPLIB_MODULES) are maintained manually. You will need to add "multiperiod_blending" to each of those lists for the PR's CI checks to cover this new module.
There was a problem hiding this comment.
Thank you for this. I have added multiperiod_blending to the test modules. @bernalde could you just check if it has been added correctly and the tests are running fine? They were working on my end but I had to install some packages like scipy, openpyxl, pint.
| return m | ||
|
|
||
|
|
||
| if __name__ == "__main__": |
There was a problem hiding this comment.
The __main__ block hardcodes gurobi as the solver. Many users and CI environments will not have Gurobi installed. A couple of options:
- Accept the solver name as a command-line argument or try several solvers in order of preference, falling back gracefully.
- At minimum, wrap the solver instantiation in a check so users get a clear error message:
if __name__ == "__main__":
import sys
instance_file = sys.argv[1] if len(sys.argv) > 1 else "instances_json/mpbp_6.json"
with open(instance_file, "r") as f:
json_obj = json.load(f)
d = convert_json_to_data(json_obj)
m = build_model(d)
pyo.TransformationFactory("core.logical_to_linear").apply_to(m)
pyo.TransformationFactory("gdp.bigm").apply_to(m)
solver_name = sys.argv[2] if len(sys.argv) > 2 else "gurobi"
opt = pyo.SolverFactory(solver_name)
if not opt.available():
raise RuntimeError(
f"Solver '${solver_name}' is not available. "
"Please install it or pass an available solver name as the second argument."
)
status = opt.solve(m, tee=True)
pyo.assert_optimal_termination(status)This also makes the instance configurable via the command line, which is handy for testing different instances.
There was a problem hiding this comment.
Thank you. I have made the change. The current command line argument is as follows:
python multiperiod_blending.py --instance instances_json/mpbp_6.json --solver gurobi
@bernalde let me know if this is not consistent with the codebase or if you would like to keep it the way you suggested above.
Changed & to &
Summary
This pull request adds a GDP model for the multiperiod blending problem (MPBP) as well as 60 instances we developed for our recent paper so that the community can use them for future GDP algorithmic benchmarking. Please feel free to suggest changes or ask any questions.
Major Changes
We have added a
multiperiod_blending/folder togdplib/. This folder contains the following files:README.mdwhich has some fundamental multiperiod blending information along with the requisite citations.instances_json/.multiperiod_blending.pyfile which contains the GDP model for the MPBP along with information on how to run a specific instance.__init__.pythat aligns with the format of other files in the gdplib repo.The multiperiod blending case has also been added to
gdplib/__init__.py.