Skip to content

include binary path - #782

Open
djps wants to merge 1 commit into
waltsims:masterfrom
djps:binary_path
Open

include binary path#782
djps wants to merge 1 commit into
waltsims:masterfrom
djps:binary_path

Conversation

@djps

@djps djps commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Closes #781

Greptile Summary

Adds a benchmark option and CLI argument for selecting a custom C++ solver binary, records that selection in benchmark metadata, and forwards it to solver execution.

  • Extends BenchmarkOptions with cpp_binary.
  • Adds --cpp-binary to the benchmark CLI.
  • Passes the selected path through native benchmark runs.

Confidence Score: 4/5

The programmatic benchmark configuration should be fixed before merging because it can ignore the configured binary and record misleading benchmark provenance.

BenchmarkOptions.cpp_binary is serialized as the selected binary, while solver execution instead uses a separately defaulted run() argument, allowing the recorded configuration and actual executable to diverge.

Files Needing Attention: benchmarks/benchmark.py, benchmarks/helpers.py

Important Files Changed

Filename Overview
benchmarks/benchmark.py Adds custom-binary parsing, recording, and solver forwarding, but keeps the configured path in two independent parameters that can diverge for programmatic callers.
benchmarks/helpers.py Adds the custom binary path to benchmark configuration and serialized options; this field is not directly used by benchmark execution.

Reviews (1): Last reviewed commit: "include binary path" | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

Comment thread benchmarks/benchmark.py
Comment on lines 112 to 116
pml_size=options.pml_size,
pml_inside=options.pml_inside,
smooth_p0=False,
binary_path=cpp_binary,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Binary configuration values diverge

When a programmatic caller sets BenchmarkOptions.cpp_binary but does not repeat it as run(cpp_binary=...), the saved options report the custom binary while execution passes None and uses the bundled binary, causing misleading benchmark provenance or a missing-binary failure.

Knowledge Base Used: Native solver integration

@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.83%. Comparing base (1744cf6) to head (ec8eebb).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #782   +/-   ##
=======================================
  Coverage   75.83%   75.83%           
=======================================
  Files          58       58           
  Lines        8274     8274           
  Branches     1614     1614           
=======================================
  Hits         6275     6275           
  Misses       1373     1373           
  Partials      626      626           
Flag Coverage Δ
3.10 75.80% <ø> (ø)
3.11 75.80% <ø> (ø)
3.12 75.80% <ø> (ø)
3.13 75.80% <ø> (ø)
macos-latest 75.71% <ø> (ø)
ubuntu-latest 75.71% <ø> (ø)
windows-latest 75.64% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

[ENHANCE] Benchmarking options

1 participant