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

[move-prover][prover-lab] Create a z3 vs cvc5 basic experiment, nuke Rust-Jupyter-Book dependency #8732

Merged
merged 1 commit into from
Jul 13, 2021

Conversation

wrwg
Copy link
Contributor

@wrwg wrwg commented Jul 13, 2021

This PR enables to compare cvc5 with z3 in benchmarks over the whole Diem framework. This is achieved in 3 steps:

  1. We remove all dependency from Jupyter Rust notebooks. These turned out to not reliably work. (It was in fact broken at head, but also before fragile and extremely slow.) Instead we provide a new prover-lab command plot which generates a .svg file (based on the same refactored plotting logic) which can be included in MD.

  2. A new lab is created under lab/data/cvc. This lab is intended to benchmark cvc5 vs z3 (later also include vector theories). The output of this lab is found in this README markdown.

  3. It turned out that some of the benchmarks lead cvc to not terminate and respect soft timeouts. To innoculate the in-process benchmark infra against such situations, we support now a 'hard timeout' option in the boogie backend. Only with this addition we can now run cvc against the full Diem framework benchmark suite. If cvc does not terminate, we mark the benchmark as error instead of
    aborting the benchmark run.

The older labs aren't yet updated to this new approach, which should be done in subsequent PRs.

Motivation

Fix bench-marking infrastructure.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

NA

Related PRs

NA

@wrwg wrwg added this to In Progress in Move Prover via automation Jul 13, 2021
@bors-libra bors-libra added this to In Review in bors Jul 13, 2021
@wrwg wrwg marked this pull request as ready for review July 13, 2021 05:52
@wrwg
Copy link
Contributor Author

wrwg commented Jul 13, 2021

During review, the correct link to the README for the cvc lab, which includes the rendered SVG, is this: https://github.com/wrwg/diem/blob/bench/language/move-prover/lab/data/cvc/README.md

ma2bd
ma2bd previously approved these changes Jul 13, 2021
Copy link
Contributor

@ma2bd ma2bd left a comment

Choose a reason for hiding this comment

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

nice!

.map(|_| ())
.map_err(|_| "expected number".to_string())
};
let cmd_line_parser = App::new("plot")
Copy link
Contributor

Choose a reason for hiding this comment

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

Here, structopt would make it easy for plot_svg to take a clean configuration structure (instead of a vector of unparsed arguments). Thanks to the "flatten" option of structopt, the configuration struct can even be equipped with its own reusable command-line parsing logic:
https://github.com/facebookincubator/smt2utils/blob/master/z3tracer/src/main.rs#L18
https://github.com/facebookincubator/smt2utils/blob/master/z3tracer/src/model.rs#L23
(^^ there is only one caveat due to a minor bug in cargo doc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm still using clap, as structopt wasn't ready when I started doing this. Should switch this and other code to it in the future.

@wrwg
Copy link
Contributor Author

wrwg commented Jul 13, 2021

/land

@bors-libra bors-libra moved this from In Review to Queued in bors Jul 13, 2021
…Rust-Jupyter-Book dependency

This PR enables to compare cvc5 with z3 in benchmarks over the whole Diem framework. This is achieved in 3 steps:

1. We remove all dependency from Jupyter Rust notebooks. These turned out to not reliably work. (It was in fact broken at head, but also before fragile and extremely slow.) Instead we provide a new prover-lab command `plot` which generates a .svg file (based on the same refactored plotting logic) which can be included in MD.

2. A new lab is created under `lab/data/cvc`. This lab is intended to benchmark cvc5 vs z3 (later also include vector theories). The output of this lab is found in this [README markdown](language/move-prover/lab/data/cvc/README.md).

3. It turned out that some of the benchmarks lead cvc to not terminate and respect soft timeouts. To innoculate the in-process benchmark infra against such situations, we support now a 'hard timeout' option in the boogie backend. Only with this addition we can now run cvc against the full Diem framework benchmark suite. If cvc does not terminate, we mark the benchmark as error instead of
aborting the benchmark run.

The older labs aren't yet updated to this new approach, which should be done in subsequent PRs.

Closes: diem#8732
@bors-libra bors-libra moved this from Queued to Testing in bors Jul 13, 2021
@github-actions
Copy link

Cluster Test Result

Test runner setup time spent 242 secs
Compatibility test results for land_6ba7b291 ==> land_b48c6336 (PR)
1. All instances running land_6ba7b291, generating some traffic on network
2. First full node land_6ba7b291 ==> land_b48c6336, to validate new full node to old validator node traffic
3. First Validator node land_6ba7b291 ==> land_b48c6336, to validate storage compatibility
4. First batch validators (14) land_6ba7b291 ==> land_b48c6336, to test consensus and traffic between old full nodes and new validator node
5. First batch full nodes (14) land_6ba7b291 ==> land_b48c6336
6. Second batch validators (15) land_6ba7b291 ==> land_b48c6336, to upgrade rest of the validators
7. Second batch of full nodes (15) land_6ba7b291 ==> land_b48c6336, to finish the network upgrade, time spent 712 secs
all up : 1033 TPS, 4394 ms latency, 4950 ms p99 latency, no expired txns, time spent 250 secs
Logs: http://kibana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-07-13T21:17:31Z',to:'2021-07-13T21:40:24Z'))
Dashboard: http://grafana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/d/performance/performance?from=1626211051000&to=1626212424000
Validator 1 logs: http://kibana.ct-1-k8s-testnet.aws.hlw3truzy4ls.com/app/kibana#/discover?_g=(time:(from:'2021-07-13T21:17:31Z',to:'2021-07-13T21:40:24Z'))&_a=(columns:!(log),query:(language:kuery,query:'kubernetes.pod_name:"val-1"'),sort:!(!('@timestamp',desc)))

Repro cmd:

./scripts/cti --tag land_6ba7b291 --cluster-test-tag land_b48c6336 -E BATCH_SIZE=15 -E UPDATE_TO_TAG=land_b48c6336 --report report.json --suite land_blocking_compat

🎉 Land-blocking cluster test passed! 👌

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Move Prover
  
Closed
Development

Successfully merging this pull request may close these issues.

None yet

3 participants