Adopt powdr's split GenerateConfig + SelectConfig + StagedPipeline#8
Open
georgwiese wants to merge 10 commits into
Open
Adopt powdr's split GenerateConfig + SelectConfig + StagedPipeline#8georgwiese wants to merge 10 commits into
georgwiese wants to merge 10 commits into
Conversation
Replace the bespoke `apc-cache/{name}.bin` blob with powdr's staged
artifacts cache. `precompute_prover_data` now calls
`StagedPipeline::setup` with per-stage cache keys chosen so a sweep
across `--apc N` values under `--pgo-type cell` reuses the candidate
ranking — `generate`'s key omits `--apc` / `--apc-skip`; `select` and
`setup` add them back.
Replaces `--apc-cache-dir` + `--apc-setup-name` with `--artifacts-dir`
(default `apc-cache`); `run.sh` updated accordingly.
powdr's `StagedPipeline::generate_apcs` now applies the `apc_candidates` default from `config.autoprecompiles + skip`, so pass the real values through. The custom `generate_key` cache string still omits apc/apc_skip, so a `--apc N` sweep under `--pgo-type cell` keeps reusing the generate-stage blob even though `powdr_config.autoprecompiles` differs.
powdr's `PowdrConfig` split into per-stage configs and `StagedPipeline` gained a typed `input_fp: &impl Hash` argument in place of the stringly-typed `cache_key`. Migrate `precompute_prover_data`: - `default_powdr_openvm_config(apc, skip)` becomes `default_powdr_openvm_configs(apc, skip) -> (GenerateConfig, SelectConfig)`. - `gen_cfg.with_select_defaults(pgo_type, select)` applies the `apc_candidates` cap default (no caller-side cap policy any more). - The three hand-rolled cache-key strings (`generate_key` / `select_key` / `setup_key`) collapse into one `input_fp = (CHAIN_ID_ETH_MAINNET, pgo_blocks.as_slice())`. The "generate key omits autoprecompiles" guarantee is now structural — `StagedPipeline::generate_apcs` only hashes `(gen, pgo, max_columns, input_fp)`; the select fields aren't in scope for it. `--apc N` sweeps under `--pgo-type cell` continue to reuse the generate blob. `gen` is a reserved keyword in edition 2024, so the local is named `gen_cfg`.
powdr's `StagedPipeline` no longer takes a stringly-typed cache_key /
input_fp; instead each stage hashes `(generate, pgo_config)` where
`pgo_config: PgoConfig { pgo_type, max_columns, inputs: Vec<u8> }`.
Adapt accordingly:
- `pgo_inputs = bincode(&pgo_stdins)` — the make_pgo_profile closure
deserializes them back instead of capturing them by reference.
- `PgoConfig::new(pgo_type, None, pgo_inputs)` becomes the single
caller-supplied piece of state for both the generate and select hash.
- The select-stage hash now includes the full PgoConfig, fixing the
upstream bug where switching `--pgo-type` would have served a stale
select-stage blob.
The empirical-constraints closure ignores its `(guest, inputs)`
arguments — openvm-eth doesn't use optimistic precompiles.
powdr's `StagedPipeline::{select_apcs, setup}` no longer take recursive
`compute_*` callbacks — they take the same `make_pgo_profile` /
`make_empirical_constraints` closures as `generate_apcs` and chain
through internally. Match the new shape: hoist the closures out of
the nested call site, pass them once to `pipeline.setup(...)`.
`make_empirical_constraints` also gained a `&GenerateConfig` parameter;
openvm-eth's variant ignores it (default empirical constraints always).
georgwiese
added a commit
to powdr-labs/powdr
that referenced
this pull request
Jun 2, 2026
PR #3763 changes the powdr autoprecompile API. The pre-existing pins target sp1 and openvm-eth commits that use the old API, so CI fails even though my refactor itself is correct. - sp1-benchmarks/Cargo.toml: bump sp1 dep branch from powdr-labs/generate-apcs-api → powdr-labs/split-powdr-config (succinctlabs/sp1#2830 — adopts the new powdr API). - .github/actions/patch-openvm-eth: bump ref from 05ed9cd3 (main) → e716b51a (powdr-labs/openvm-eth#8 tip, adopts the new powdr API). The "Verify openvm-eth ref is on main" check will (intentionally) fail until openvm-eth#8 merges — the comment in pr-tests-with-secrets.yml already accounts for this case, and the actual reth test runs against the compatible code.
7 tasks
The StagedPipeline refactor dropped the `POWDR_APC_CANDIDATES_DIR` wiring in `precompute_prover_data`, so `GenerateConfig::apc_candidates_dir_path` stayed `None` and the combined `apc_candidates.json` was never written. The powdr nightly's reth bench fails its final `mv apcs/apc_candidates.json` as a result. Re-add the env-var wiring (mirroring the pre-refactor `compile_apc_program`) so the `generate` stage dumps per-candidate JSON/TXT + the combined `apc_candidates.json` again. It's a side effect of the generate stage, so it refreshes on a generate cache miss — fine for the bench, which runs against a fresh apc-cache each time. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Hoist the provider construction out of the pgo-blocks guard and flatten the stdin loop; chain `with_select_defaults` onto `default_generate_config` (keeping the `POWDR_APC_CANDIDATES_DIR` opt-in); and replace the empirical-constraints closure with a named `make_default_empirical_constraints`. The named fn references `GenerateConfig` unqualified, so import it from `powdr_autoprecompiles` (the replaced closure had used the fully-qualified path); without the import the crate fails to build. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`StagedPipeline` moved from `powdr-openvm-riscv` down into `powdr-openvm` (now generic over `OpenVmISA`). Import it from there, and replace the local `make_default_empirical_constraints` with the shared helper now exported by `powdr-openvm` for callers that don't use optimistic precompiles. Co-Authored-By: Claude Opus 4.8 (1M context) <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
1 participant
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.
Summary
Adapts to powdr-labs/powdr#3763. Note that this removes a lot of the custom caching implemented in this repository: Via the use of
StagedPipelinefrompowdr_openvmwith an artifacts dir, APCs and setup is already cached.