Gurobi auto-detection, working graphs without it - #798
Open
oflatt wants to merge 36 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes the nightly pipeline work in a “CBC-only” mode by auto-detecting whether Gurobi is installed/licensed, and gating graph generation based on what timing data is actually present rather than a single boolean flag.
Changes:
- Add Gurobi auto-detection + new
--no-gurobiflag, and adjust profiling/treatments so the COMPARISON run still produces useful CBC/tiger timing data without Gurobi. - Gate ILP/statewalk/region-timing graphs on available COMPARISON/Gurobi data; update normalized charts to label ILP series as CBC vs Gurobi.
- Add docs + tooling for setup (Ubuntu CBC binary install notes,
infra/requirements.txt,make gurobi-setup,infra/setup_gurobi.sh).
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Documents CBC binary dependency, local nightly prerequisites, and optional Gurobi behavior/flags. |
| Makefile | Adds gurobi-setup target to install/verify a Gurobi license. |
| install_ubuntu.sh | Installs both CBC dev library and cbc CLI binary; clarifies the distinction. |
| infra/statewalk_graphs.py | Allows single-treatment plotting for multi scatter (needed for CBC-only mode). |
| infra/setup_gurobi.sh | New helper script to install/verify a Gurobi license via gurobi_cl. |
| infra/requirements.txt | Adds pinned Python deps for nightly graph generation. |
| infra/profile.py | Adds Gurobi availability probe, moves COMPARISON into base treatments, and adjusts ILP timing flags for CBC-only. |
| infra/nightly.py | Adds --no-gurobi, implements auto-detection, and enforces flag combinations. |
| infra/graphs.py | Gates graph generation based on data presence; supports CBC-only normalized charts and labeling. |
| infra/graph_helpers.py | Adds helpers to detect whether COMPARISON ran and whether Gurobi timing data exists. |
| infra/extract_time_graph.py | Makes extraction CDF optionally omit the Gurobi series. |
| dag_in_context/src/tiger/time_ilp.h | Adds ilp_ran to mark when Gurobi timing was skipped. |
| dag_in_context/src/tiger/time_ilp.cpp | Skips populating ilp_* fields when Gurobi was skipped; emits ilp_ran to JSON. |
| dag_in_context/src/tiger/main.h | Adds config flag for whether to run Gurobi during --time-ilp. |
| dag_in_context/src/tiger/main.cpp | Disables the Gurobi timing run when --ilp-solver cbc is selected. |
| dag_in_context/src/lib.rs | Adds ilp_ran to Rust-side deserialization with defaults for older data. |
| .gitignore | Ignores gurobi.log. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| set -euo pipefail | ||
|
|
||
| LICENSE_SRC="${1:-${GUROBI_LICENSE_FILE:-}}" |
Comment on lines
+36
to
+42
| if [ "$src_abs" != "$dest_abs" ]; then | ||
| cp "$LICENSE_SRC" "$DEST" | ||
| echo "Copied license to $DEST (Gurobi finds this automatically)." | ||
| else | ||
| echo "License already installed at $DEST." | ||
| fi | ||
| echo "(To keep the license elsewhere, set GRB_LICENSE_FILE in your shell profile instead.)" |
The CBC ILP treatment can exceed the wall-clock timeout (or be infeasible) on large benchmarks, producing no cycles. The normalized chart substitutes that treatment when Gurobi is absent, so guard every normalized() access with has_run_cycles() and mark treatments with no result instead of averaging an empty cycles list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
In non-paper runs the eggcc-tiger-ILP-COMPARISON treatment can hit the wall-clock timeout and produce no region timings (extractRegionTimings == False). make_macros assumed every benchmark had a list of samples and crashed iterating a bool. Filter the benchmark list to those with valid comparison timings up front; in paper mode (where the comparison always succeeds) this filters nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The no-Gurobi nightly timed out at the server's 3h limit because CBC ILP solving burns up to the 5-min per-region timeout on hard regions, across every benchmark (both the ILP-CBC extraction treatment and the now-default COMPARISON pass). Make the per-region ILP timeout configurable (eggcc --ilp-timeout-seconds -> tiger), default 300s. NightlyConfig uses 30s in the default (non-paper) nightly and keeps 5min for paper. Graphs now report the actual timeout used (e.g. 'Gurobi Timeout (30 s)') instead of a hardcoded '5 min', via a run-set value in graph_helpers. Measured: a 2mm COMPARISON run drops from 340s to ~20s. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…e Makefile arg - Accept the standard GRB_LICENSE_FILE (fallback GUROBI_LICENSE_FILE) as the source license path, and advertise GRB_LICENSE_FILE in usage so it matches the script's own messages. - chmod 600 the installed ~/gurobi.lic (license files can hold WLS secrets). - Quote $(LICENSE) in the Makefile target so paths with spaces aren't split. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
artifact/ directory for the OOPSLA/SPLASH artifact evaluation: - provision.sh: turns a fresh Ubuntu 22.04 into the built artifact (LLVM 18, CBC, Rust, pinned Python deps in a hidden venv, tokei, graphviz), clones+builds eggcc, sets up a clean reviewer home, and pre-generates the figures. - build_vm.sh: macOS/VirtualBox 7.2 driver (unattended Ubuntu install + provision over SSH). Untested (no VirtualBox in the build env); README has a manual fallback. - reproduce.sh: smoke|full|paper -> the CDF of ILP vs tiger extraction times plus the normalized bar charts (bril/polybench/fenwick/raytrace) and the Fenwick case study. Figures are copied to a clean top-level dir; smoke uses a distinct filename so it never clobbers the shipped full figures. - infra/plot_cdf.py: generates just the CDF from a profile.json (subset-friendly). - README.md: AE-structured, with core/time estimates (4 vCPU VM; smoke ~5-10 min, full ~3-4 h) and a note that 100%-region paper runs need a large machine (~100 cores). All eval scripts are configured via command-line flags (no config env vars). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
oflatt
force-pushed
the
oflatt-gurobi-optional
branch
from
July 13, 2026 20:10
2c5bdf1 to
1864d61
Compare
* Add Docker packaging for the artifact (alternative to the VM) VirtualBox 7.2 on Apple-Silicon Macs can only run ARM guests, so an amd64 artifact VM can't be built there. Add a Docker image as an alternative packaging path (which the SPLASH guidance accepts). - infra/Dockerfile: mirrors the tested provision.sh (LLVM 18, CBC, Rust 1.88.0 + nightly-2024-05-01 for the runtime, cargo build, pinned python venv) minus the VM-only GNOME desktop; ships a ./reproduce.sh that writes figures to a bind-mounted /out. - infra/build_docker.sh: build driver (single- and multi-platform via buildx), the Docker counterpart to build_vm.sh. - BUILDING.md: how the image is built + multi-platform + ship-to-Zenodo. - artifact/README.md: reviewer-facing "run via Docker" instructions. Note: the image build was not executed in the authoring environment (no container runtime available), but each step mirrors the tested provision.sh. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Make Docker the artifact; remove the VirtualBox VM packaging We're shipping the artifact as Docker instead of a VM, so remove the VM packaging entirely rather than keeping both: - delete infra/build_vm.sh and infra/provision.sh - rewrite infra/BUILDING.md and artifact/README.md as Docker-first (no VirtualBox / OVA / ISO); reviewers load or pull the image and run reproduce.sh with a bind-mounted /out for the figures - drop now-stale VM references from the Dockerfile, build_docker.sh, and reproduce.sh comments Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Oliver Flatt <oflatt@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
PR #799 was merged prematurely as its Docker-only state (commit b193a31), which removed build_vm.sh, provision.sh, and the VM-centric docs. VirtualBox works now, so this reverts that merge to bring the VM approach back. Co-authored-by: Oliver Flatt <oflatt@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.