Skip to content

Fix methanol GLOA benchmark summary generation - #128

Open
bernalde wants to merge 2 commits into
mainfrom
fix/issue-70-methanol-gloa
Open

bernalde wants to merge 2 commits into
mainfrom
fix/issue-70-methanol-gloa

Conversation

@bernalde

Copy link
Copy Markdown
Member

Summary

  • Package the benchmark summary generator as gdplib.benchmark_summary so the installed gdplib-benchmark entry point can generate summaries without importing a repo-root-only script.
  • Keep generate_benchmark_summary_all.py as a compatibility wrapper around the packaged implementation.
  • Add a solver-free regression test using a methanol gdpopt.gloa result JSON to cover summary generation from benchmark result folders.
  • Considered Plan PyPI release workflow and Pixi platform support #104: this PR does not change release workflow, Pixi platform policy, or pixi.lock.

Tests run

  • /home/bernalde/.pixi/bin/pixi run pytest tests/test_benchmark.py::test_generate_summary_uses_packaged_summary_module -v --tb=short - passed.
  • /home/bernalde/.pixi/bin/pixi run pytest tests/test_methanol.py -v --tb=short - passed, 3 tests.
  • /home/bernalde/.pixi/bin/pixi run pytest 'tests/test_module_imports.py::TestModelConstruction::test_model_construction[methanol]' -v --tb=short - passed.
  • /home/bernalde/.pixi/bin/pixi run pytest tests/test_benchmark.py -v --tb=short - passed, 18 tests.
  • /home/bernalde/.pixi/bin/pixi run test - passed, 282 passed and 1 skipped.
  • /home/bernalde/.pixi/bin/pixi run lint - passed. The non-blocking flake8 --exit-zero step still reports existing repository warnings.
  • /home/bernalde/.pixi/bin/pixi run gdplib-benchmark run --instances methanol --strategies gdpopt.gloa --timelimit 60 --solver-profile gams-baron --run-id issue70_methanol_gloa_final_verify - passed with 1 result row, 0 failures, summary generated. Solver result: optimal, objective -1743.4292381783366.

Notes about tests not run

  • I did not run the full PR58 benchmark matrix; this PR was verified with the focused methanol GLOA benchmark path because the full matrix is solver-backed and much broader than issue Refactor methanol to fix benchmark issues #70.

Closes #70

@bernalde

Copy link
Copy Markdown
Member Author

Follow-up on the methanol GLOA correctness question:

This PR fixes the benchmark summary import path so the installed gdplib-benchmark entry point can complete summary generation. It does not change the methanol model or GDPopt behavior.

I tested the focused methanol GLOA path against Pyomo/pyomo#3940, which fixes GLOA convex affine cut generation:

  • Current GDPlib Pixi Pyomo 6.10.0, command /home/bernalde/.pixi/bin/pixi run gdplib-benchmark run --instances methanol --strategies gdpopt.gloa --timelimit 60 --solver-profile gams-baron --run-id issue70_methanol_gloa_final_verify
    • termination: optimal
    • iterations: 1
    • objective: -1743.4292381783366
  • Fix GLOA convex affine cut slope Pyomo/pyomo#3940 loaded via PYTHONPATH=/tmp/pyomo-pr3940, command PYTHONPATH=/tmp/pyomo-pr3940 /home/bernalde/.pixi/bin/pixi run gdplib-benchmark run --instances methanol --strategies gdpopt.gloa --timelimit 60 --solver-profile gams-baron --run-id issue70_methanol_gloa_pyomo3940 --no-summary
    • termination: optimal
    • iterations: 15
    • objective: -1793.4292385308854

The patched Pyomo result matches the documented methanol best-known objective -1793.4292381783 to solver tolerance. That strongly suggests the remaining methanol GLOA global-solution mismatch in #70 is a Pyomo GLOA affine-cut bug rather than a GDPlib methanol formulation bug.

Implication for this PR: #128 is still useful because it makes the benchmark CLI complete successfully after a methanol GLOA run, but we should not claim the current GDPlib dependency stack fixes GLOA optimality until the Pyomo fix is merged and available in the environment used for verification.

@bernalde
bernalde force-pushed the fix/issue-70-methanol-gloa branch from ef4b00c to 58c0961 Compare May 14, 2026 16:19
@bernalde

Copy link
Copy Markdown
Member Author

Objective-sense note (context: Pyomo/pyomo#3986, merged 2026-08-13, fixed the GDPopt LOA objective-sense bug tracked in #114):

No changes needed here — this PR's scope is GLOA benchmark summary generation, and the fix was LOA-only (GLOA passes the objective sense through correctly), so the verification evidence stands. The related follow-up — reverting methanol's minimize(-profit) workaround to native maximize(profit) and updating the README note — is tracked in #70 for after a Pyomo release containing the fix, and is intentionally out of scope for this PR.

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.

Refactor methanol to fix benchmark issues

1 participant