Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use parallel prover branch #520

Merged
merged 25 commits into from
May 10, 2024
Merged

Use parallel prover branch #520

merged 25 commits into from
May 10, 2024

Conversation

nwatson22
Copy link
Member

@nwatson22 nwatson22 commented Apr 18, 2024

  • Use parallel prover branch evm-semantics#2400

  • Parallel proof exploration k#4256

  • Enables single-proof parallelism on foundry_prove, which makes parallel_advance_proof the default on the kontrol test suite and the kontrol prove CLI.

  • To use, include the --max-frontier-parallel N option with kontrol prove to use up to N worker threads to advance independent proof branches in parallel.

  • To streamline the creation of a create_kcfg_explore function to pass to KEVM's run_prover, uses kore_server in _run_cfg_group instead of legacy_explore (so we can get access to the server's port instead of having to copy the port of the client buried deep within the KCFGExplore.

  • To clean up the logic of choosing between starting a new KoreServer and using the port of a KoreServer that was already started elsewhere, introduces OptionalKoreServer and 2 subclasses for each case that each yield the server port and handle closing the underlying server (if applicable).

@nwatson22 nwatson22 self-assigned this Apr 19, 2024
rv-jenkins pushed a commit to runtimeverification/k that referenced this pull request May 2, 2024
### Additions:

- Adds `ParallelProver`, which uses `ProverPool` to handle a
`ThreadPoolExecutor` that creates a new prover for each new thread used
(based on `KoreServerPool`)
- `ParallelProver.parallel_advance_proof` executes a single proof in
parallel.
- Requires parameter `create_prover: Callable[[], Prover[P, PS, SR]]` to
know how to create a fresh `Prover`.
- User is responsible for making sure this function produces `Provers`
which are independent, i.e. don't refer to the same `KoreClient`, etc.
(Although they can/should send requests to the same server).
- Reworks `APRProofStep` to no longer contain a proof reference, but
instead only immutable data.
- Moves the prior loops counting and zero depth bound checking to the
`get_steps` stage, because they require whole-KCFG knowledge
- Adds an example of how to use parallel proving in
`test_all_path_reachability_prove_parallel`.
- Adds a test `long-branches` in `imp-simple-spec` intended for
profiling parallelization.

### Remaining issues:

- ~~Adds an `id()` abstract method to `ProofStep`, to be able to
differentiate steps (TODO this might be able to be removed now if we can
enforce `ProofStep` being a hashable dataclass)~~
- Saves the `port` as a field in `KoreClient`. This can probably be
removed or improved somehow. I'm just using it downstream to get the
port being used by a `Prover` in order to generate fresh `Prover`s with
the same port, when we don't have prior access to that port, i.e.
`KoreServer` was allowed to choose its own port. The port is already
accessible through `KoreClient`, but you have to access private vars
several layers deep.
- ~~Adds `kcfg_semantics` as parameter to `get_steps`, because I haven't
been able to find a way to separate the prior loops counting algorithm
into two parts, one with `kcfg` and one with `same_loop`, without
breaking the prior loop caching. This is not ideal, because
`kcfg_explore` probably shouldn't even be in `Prover`, just subclasses
of `Prover` that deal with `KCFG`s.~~
- ~~To actually get the benefit of parallelization, you need to set
`GHC='-N[threads]'` so the single shared `KoreServer` can start its own
threads to deal with requests in parallel. This should probably be
automated, but I'm not sure how best to do that.~~
- Perhaps a better design than passing around a `create_prover` function
would be to figure out all relevant parameters for doing this and
passing them to `ParallelProver()`.
- ~~Maybe move parameters from `parallel_advance_proof` to
`ParallelProver.__init__()`~~
- The function passing design currently also makes it so that we aren't
using the contextmanager features of `KoreServer`, so ~~the KoreServers
probably aren't getting closed properly~~.
- I would like others to check my work re. thread safety.

### Tested on kevm and kontrol:

- runtimeverification/evm-semantics#2400
- runtimeverification/kontrol#520

---------

Co-authored-by: Palina Tolmach <[email protected]>
@nwatson22 nwatson22 mentioned this pull request May 2, 2024
@nwatson22 nwatson22 marked this pull request as ready for review May 10, 2024 03:05
src/kontrol/prove.py Show resolved Hide resolved
src/kontrol/prove.py Outdated Show resolved Hide resolved
@nwatson22 nwatson22 requested a review from ehildenb May 10, 2024 17:22
@nwatson22 nwatson22 added enhancement New feature or request automerge labels May 10, 2024
@rv-jenkins rv-jenkins merged commit 3c5a436 into master May 10, 2024
12 checks passed
@rv-jenkins rv-jenkins deleted the noah/parallel-integration branch May 10, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants