From 12919853aee70cebe2721e1477174e151c502dd4 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 11:28:14 +0100 Subject: [PATCH 01/13] Pull CSV export into pipeline --- compiler/src/pipeline.rs | 47 +++++++++++++++++++++++++++++++++++++++- number/src/serialize.rs | 11 ++++++++-- powdr_cli/src/main.rs | 45 +++++++------------------------------- 3 files changed, 63 insertions(+), 40 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index c7c7ea83d..d65c1f378 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -1,6 +1,7 @@ use std::{ fmt::Display, fs, + io::BufWriter, path::{Path, PathBuf}, time::Instant, }; @@ -20,7 +21,7 @@ use executor::{ }; use log::Level; use mktemp::Temp; -use number::FieldElement; +use number::{write_polys_csv_file, CsvRenderMode, FieldElement}; use crate::{ inputs_to_query_callback, @@ -106,6 +107,8 @@ struct Arguments { external_witness_values: Vec<(String, Vec)>, query_callback: Option>>, backend: Option, + csv_render_mode: CsvRenderMode, + export_witness_csv: bool, } pub struct Pipeline { @@ -218,6 +221,20 @@ impl Pipeline { ..self } } + pub fn with_witness_csv_settings( + self, + export_witness_csv: bool, + csv_render_mode: CsvRenderMode, + ) -> Self { + Pipeline { + arguments: Arguments { + csv_render_mode, + export_witness_csv, + ..self.arguments + }, + ..self + } + } pub fn add_query_callback(self, query_callback: Box>) -> Self { let query_callback = match self.arguments.query_callback { @@ -398,6 +415,7 @@ impl Pipeline { .map(|(name, c)| (name.to_string(), c)) .collect::>() }); + self.maybe_write_witness_csv(&constants, &witness)?; Artifact::GeneratedWitness(GeneratedWitness { pil, constants, @@ -473,6 +491,33 @@ impl Pipeline { Ok(()) } + fn maybe_write_witness_csv( + &self, + fixed: &[(String, Vec)], + witness: &Option)>>, + ) -> Result<(), Vec> { + if !self.arguments.export_witness_csv { + return Ok(()); + } + + if let Some(output_dir) = &self.output_dir { + let columns = fixed + .iter() + .chain(match witness.as_ref() { + Some(witness) => witness.iter(), + None => [].iter(), + }) + .collect::>(); + + let csv_path = Path::new(output_dir).join("columns.csv"); + let mut csv_file = fs::File::create(csv_path).map_err(|e| vec![format!("{}", e)])?; + let mut csv_writer = BufWriter::new(&mut csv_file); + + write_polys_csv_file(&mut csv_writer, self.arguments.csv_render_mode, &columns); + } + Ok(()) + } + fn stage(&self) -> Stage { match self.artifact.as_ref().unwrap() { Artifact::AsmFile(_) => Stage::AsmFile, diff --git a/number/src/serialize.rs b/number/src/serialize.rs index 916c34721..22cf224d6 100644 --- a/number/src/serialize.rs +++ b/number/src/serialize.rs @@ -11,12 +11,18 @@ pub enum CsvRenderMode { Hex, } +impl Default for CsvRenderMode { + fn default() -> Self { + Self::Hex + } +} + const ROW_NAME: &str = "Row"; pub fn write_polys_csv_file( file: &mut impl Write, render_mode: CsvRenderMode, - polys: &[(String, Vec)], + polys: &[&(String, Vec)], ) { let mut writer = Writer::from_writer(file); @@ -172,6 +178,7 @@ mod tests { .into_iter() .map(|(name, values)| (name.to_string(), values)) .collect::>(); + let polys_ref = polys.iter().collect::>(); for render_mode in &[ CsvRenderMode::SignedBase10, @@ -179,7 +186,7 @@ mod tests { CsvRenderMode::Hex, ] { let mut buf: Vec = vec![]; - write_polys_csv_file(&mut buf, *render_mode, &polys); + write_polys_csv_file(&mut buf, *render_mode, &polys_ref); let read_polys = read_polys_csv_file::(&mut Cursor::new(buf)); assert_eq!(read_polys, polys); diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index acaab6a50..8829ffd64 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -10,7 +10,7 @@ use env_logger::fmt::Color; use env_logger::{Builder, Target}; use log::LevelFilter; use number::write_polys_file; -use number::{read_polys_csv_file, write_polys_csv_file, CsvRenderMode}; +use number::{read_polys_csv_file, CsvRenderMode}; use number::{Bn254Field, FieldElement, GoldilocksField}; use riscv::continuations::{rust_continuations, rust_continuations_dry_run}; use riscv::{compile_riscv_asm, compile_rust}; @@ -678,10 +678,17 @@ fn compile_with_csv_export( let output_dir = Path::new(&output_directory); + let csv_mode = match csv_mode { + CsvRenderModeCLI::SignedBase10 => CsvRenderMode::SignedBase10, + CsvRenderModeCLI::UnsignedBase10 => CsvRenderMode::UnsignedBase10, + CsvRenderModeCLI::Hex => CsvRenderMode::Hex, + }; + let mut pipeline = Pipeline::default() .with_output(output_dir.to_path_buf(), force) .from_file(PathBuf::from(file)) .with_external_witness_values(external_witness_values) + .with_witness_csv_settings(export_csv, csv_mode) .with_prover_inputs(split_inputs(&inputs)); pipeline.advance_to(Stage::GeneratedWitness).unwrap(); @@ -700,45 +707,9 @@ fn compile_with_csv_export( } } - if export_csv { - // Compilation result is None if the ASM file has not been compiled - // (e.g. it has been compiled before and the force flag is not set) - if let Some(compilation_result) = result { - let csv_path = Path::new(&output_directory).join("columns.csv"); - export_columns_to_csv::( - compilation_result.constants, - compilation_result.witness, - &csv_path, - csv_mode, - ); - } - } Ok(()) } -fn export_columns_to_csv( - fixed: Vec<(String, Vec)>, - witness: Option)>>, - csv_path: &Path, - render_mode: CsvRenderModeCLI, -) { - let columns = fixed - .into_iter() - .chain(witness.unwrap_or(vec![])) - .collect::>(); - - let mut csv_file = fs::File::create(csv_path).unwrap(); - let mut csv_writer = BufWriter::new(&mut csv_file); - - let render_mode = match render_mode { - CsvRenderModeCLI::SignedBase10 => CsvRenderMode::SignedBase10, - CsvRenderModeCLI::UnsignedBase10 => CsvRenderMode::UnsignedBase10, - CsvRenderModeCLI::Hex => CsvRenderMode::Hex, - }; - - write_polys_csv_file(&mut csv_writer, render_mode, &columns); -} - fn read_and_prove( file: &Path, dir: &Path, From 54a57919028e6f6217bcfd518a90efd7ccbfd2e8 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 12:06:59 +0100 Subject: [PATCH 02/13] Pull more serialization into pipeline --- compiler/src/pipeline.rs | 107 ++++++++++++++++++++++++++------------- compiler/src/verify.rs | 31 +----------- powdr_cli/src/main.rs | 68 +++---------------------- 3 files changed, 80 insertions(+), 126 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index d65c1f378..1ec93ab18 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -1,7 +1,7 @@ use std::{ fmt::Display, fs, - io::BufWriter, + io::{BufWriter, Write}, path::{Path, PathBuf}, time::Instant, }; @@ -21,12 +21,9 @@ use executor::{ }; use log::Level; use mktemp::Temp; -use number::{write_polys_csv_file, CsvRenderMode, FieldElement}; +use number::{write_polys_csv_file, write_polys_file, CsvRenderMode, FieldElement}; -use crate::{ - inputs_to_query_callback, - verify::{write_commits_to_fs, write_constants_to_fs, write_constraints_to_fs}, -}; +use crate::inputs_to_query_callback; pub struct GeneratedWitness { pub pil: Analyzed, @@ -389,7 +386,8 @@ impl Pipeline { let constants = constants .into_iter() .map(|(k, v)| (k.to_string(), v)) - .collect(); + .collect::>(); + self.maybe_write_constants(&constants)?; self.log(&format!("Took {}", start.elapsed().as_secs_f32())); Artifact::PilWithConstants(PilWithConstants { pil, constants }) } @@ -415,7 +413,8 @@ impl Pipeline { .map(|(name, c)| (name.to_string(), c)) .collect::>() }); - self.maybe_write_witness_csv(&constants, &witness)?; + + self.maybe_write_witness(&constants, &witness)?; Artifact::GeneratedWitness(GeneratedWitness { pil, constants, @@ -444,22 +443,16 @@ impl Pipeline { None, ); - if let Some(output_dir) = &self.output_dir { - write_constants_to_fs(&constants, output_dir); - if let Some(witness) = &witness { - write_commits_to_fs(witness, output_dir); - } - if let Some(constraints_serialization) = &constraints_serialization { - write_constraints_to_fs(constraints_serialization, output_dir); - } - }; - - Artifact::Proof(ProofResult { + let proof_result = ProofResult { constants, witness, proof, constraints_serialization, - }) + }; + + self.maybe_wite_proof(&proof_result)?; + + Artifact::Proof(proof_result) } Artifact::Proof(_) => panic!("Last pipeline step!"), }); @@ -491,29 +484,71 @@ impl Pipeline { Ok(()) } - fn maybe_write_witness_csv( + fn maybe_write_constants(&self, constants: &[(String, Vec)]) -> Result<(), Vec> { + if let Some(output_dir) = &self.output_dir { + let to_write = output_dir.join("constants.bin"); + write_polys_file( + &mut BufWriter::new(&mut fs::File::create(&to_write).unwrap()), + constants, + ); + log::info!("Wrote {}.", to_write.display()); + } + Ok(()) + } + + fn maybe_write_witness( &self, fixed: &[(String, Vec)], witness: &Option)>>, ) -> Result<(), Vec> { - if !self.arguments.export_witness_csv { - return Ok(()); - } - if let Some(output_dir) = &self.output_dir { - let columns = fixed - .iter() - .chain(match witness.as_ref() { - Some(witness) => witness.iter(), - None => [].iter(), - }) - .collect::>(); + if let Some(witness) = witness.as_ref() { + let to_write = output_dir.join("commits.bin"); + write_polys_file( + &mut BufWriter::new(&mut fs::File::create(&to_write).unwrap()), + witness, + ); + log::info!("Wrote {}.", to_write.display()); + } - let csv_path = Path::new(output_dir).join("columns.csv"); - let mut csv_file = fs::File::create(csv_path).map_err(|e| vec![format!("{}", e)])?; - let mut csv_writer = BufWriter::new(&mut csv_file); + if self.arguments.export_witness_csv { + let columns = fixed + .iter() + .chain(match witness.as_ref() { + Some(witness) => witness.iter(), + None => [].iter(), + }) + .collect::>(); - write_polys_csv_file(&mut csv_writer, self.arguments.csv_render_mode, &columns); + let csv_path = Path::new(output_dir).join("columns.csv"); + let mut csv_file = + fs::File::create(&csv_path).map_err(|e| vec![format!("{}", e)])?; + let mut csv_writer = BufWriter::new(&mut csv_file); + + write_polys_csv_file(&mut csv_writer, self.arguments.csv_render_mode, &columns); + log::info!("Wrote {}.", csv_path.display()); + } + } + Ok(()) + } + + fn maybe_wite_proof(&self, proof_result: &ProofResult) -> Result<(), Vec> { + if let Some(output_dir) = &self.output_dir { + if let Some(constraints_serialization) = &proof_result.constraints_serialization { + let to_write = output_dir.join("constraints.json"); + let mut file = fs::File::create(&to_write).unwrap(); + file.write_all(constraints_serialization.as_bytes()) + .unwrap(); + log::info!("Wrote {}.", to_write.display()); + } + if let Some(proof) = &proof_result.proof { + // No need to bufferize the writing, because we write the whole + // proof in one call. + let to_write = output_dir.join("proof.bin"); + let mut proof_file = fs::File::create(&to_write).unwrap(); + proof_file.write_all(proof).unwrap(); + log::info!("Wrote {}.", to_write.display()); + } } Ok(()) } diff --git a/compiler/src/verify.rs b/compiler/src/verify.rs index 606a686cb..99c1b0190 100644 --- a/compiler/src/verify.rs +++ b/compiler/src/verify.rs @@ -1,33 +1,4 @@ -use number::write_polys_file; -use number::FieldElement; -use std::{ - fs, - io::{BufWriter, Write}, - path::Path, - process::Command, -}; - -pub fn write_constants_to_fs(constants: &[(String, Vec)], output_dir: &Path) { - let to_write = output_dir.join("constants.bin"); - write_polys_file( - &mut BufWriter::new(&mut fs::File::create(to_write).unwrap()), - constants, - ); -} - -pub fn write_commits_to_fs(commits: &[(String, Vec)], output_dir: &Path) { - let to_write = output_dir.join("commits.bin"); - write_polys_file( - &mut BufWriter::new(&mut fs::File::create(to_write).unwrap()), - commits, - ); -} - -pub fn write_constraints_to_fs(constraints: &String, output_dir: &Path) { - let to_write = output_dir.join("constraints.json"); - let mut file = fs::File::create(to_write).unwrap(); - file.write_all(constraints.as_bytes()).unwrap(); -} +use std::{path::Path, process::Command}; pub fn verify(temp_dir: &Path) { let pilcom = std::env::var("PILCOM") diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index 8829ffd64..1b77dbe9f 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -4,12 +4,11 @@ mod util; use backend::{Backend, BackendType, Proof}; use clap::{CommandFactory, Parser, Subcommand}; -use compiler::pipeline::{Pipeline, ProofResult, Stage}; +use compiler::pipeline::{Pipeline, Stage}; use compiler::util::{read_poly_set, FixedPolySet, WitnessPolySet}; use env_logger::fmt::Color; use env_logger::{Builder, Target}; use log::LevelFilter; -use number::write_polys_file; use number::{read_polys_csv_file, CsvRenderMode}; use number::{Bn254Field, FieldElement, GoldilocksField}; use riscv::continuations::{rust_continuations, rust_continuations_dry_run}; @@ -692,20 +691,7 @@ fn compile_with_csv_export( .with_prover_inputs(split_inputs(&inputs)); pipeline.advance_to(Stage::GeneratedWitness).unwrap(); - let result = prove_with.map(|backend| pipeline.with_backend(backend).proof().unwrap()); - - if let Some(ref compilation_result) = result { - serialize_result_witness(output_dir, compilation_result); - - if let Some(_backend) = &prove_with { - write_proving_results_to_fs( - false, - &compilation_result.proof, - &compilation_result.constraints_serialization, - output_dir, - ); - } - } + prove_with.map(|backend| pipeline.with_backend(backend).proof().unwrap()); Ok(()) } @@ -727,9 +713,10 @@ fn read_and_prove( assert_eq!(fixed.1, witness.1); + // TODO: Pull this into pipeline let builder = backend_type.factory::(); let backend = if let Some(filename) = params { - let mut file = fs::File::open(dir.join(filename)).unwrap(); + let mut file = fs::File::open(filename).unwrap(); builder.create_from_setup(&mut file).unwrap() } else { builder.create(fixed.1) @@ -737,7 +724,7 @@ fn read_and_prove( let proof = proof_path.map(|filename| { let mut buf = Vec::new(); - fs::File::open(dir.join(filename)) + fs::File::open(filename) .unwrap() .read_to_end(&mut buf) .unwrap(); @@ -745,8 +732,8 @@ fn read_and_prove( }); let is_aggr = proof.is_some(); - let (proof, constraints_serialization) = backend.prove(&pil, &fixed.0, &witness.0, proof); - write_proving_results_to_fs(is_aggr, &proof, &constraints_serialization, dir); + let (proof, _) = backend.prove(&pil, &fixed.0, &witness.0, proof); + write_proving_results_to_fs(is_aggr, &proof, dir); } #[allow(clippy::print_stdout)] @@ -760,36 +747,7 @@ fn optimize_and_output(file: &str) { ); } -fn serialize_result_witness(output_dir: &Path, results: &ProofResult) { - write_constants_to_fs(&results.constants, output_dir); - let witness = results.witness.as_ref().unwrap(); - write_commits_to_fs(witness, output_dir); -} - -fn write_constants_to_fs(constants: &[(String, Vec)], output_dir: &Path) { - let to_write = output_dir.join("constants.bin"); - write_polys_file( - &mut BufWriter::new(&mut fs::File::create(&to_write).unwrap()), - constants, - ); - log::info!("Wrote {}.", to_write.display()); -} - -fn write_commits_to_fs(commits: &[(String, Vec)], output_dir: &Path) { - let to_write = output_dir.join("commits.bin"); - write_polys_file( - &mut BufWriter::new(&mut fs::File::create(&to_write).unwrap()), - commits, - ); - log::info!("Wrote {}.", to_write.display()); -} - -fn write_proving_results_to_fs( - is_aggregation: bool, - proof: &Option, - constraints_serialization: &Option, - output_dir: &Path, -) { +fn write_proving_results_to_fs(is_aggregation: bool, proof: &Option, output_dir: &Path) { match proof { Some(proof) => { let fname = if is_aggregation { @@ -807,16 +765,6 @@ fn write_proving_results_to_fs( } None => log::warn!("No proof was generated"), } - - match constraints_serialization { - Some(json) => { - let to_write = output_dir.join("constraints.json"); - let mut file = fs::File::create(&to_write).unwrap(); - file.write_all(json.as_bytes()).unwrap(); - log::info!("Wrote {}.", to_write.display()); - } - None => log::warn!("Constraints were not JSON serialized"), - } } #[cfg(test)] From 522efdca4a2d401901d40ed26731f5fc19b30fdd Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 14:00:31 +0100 Subject: [PATCH 03/13] Pull aggregation into pipeline --- compiler/src/pipeline.rs | 76 ++++++++++++++++++++++++++++++++++++---- powdr_cli/src/main.rs | 60 +++++-------------------------- 2 files changed, 78 insertions(+), 58 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index 1ec93ab18..52abba2a5 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -1,7 +1,7 @@ use std::{ fmt::Display, fs, - io::{BufWriter, Write}, + io::{BufWriter, Read, Write}, path::{Path, PathBuf}, time::Instant, }; @@ -23,7 +23,10 @@ use log::Level; use mktemp::Temp; use number::{write_polys_csv_file, write_polys_file, CsvRenderMode, FieldElement}; -use crate::inputs_to_query_callback; +use crate::{ + inputs_to_query_callback, + util::{read_poly_set, FixedPolySet, WitnessPolySet}, +}; pub struct GeneratedWitness { pub pil: Analyzed, @@ -106,6 +109,8 @@ struct Arguments { backend: Option, csv_render_mode: CsvRenderMode, export_witness_csv: bool, + setup_file: Option, + existing_proof_file: Option, } pub struct Pipeline { @@ -261,6 +266,26 @@ impl Pipeline { } } + pub fn with_setup_file(self, setup_file: Option) -> Self { + Pipeline { + arguments: Arguments { + setup_file, + ..self.arguments + }, + ..self + } + } + + pub fn with_existing_proof_file(self, existing_proof_file: Option) -> Self { + Pipeline { + arguments: Arguments { + existing_proof_file, + ..self.arguments + }, + ..self + } + } + pub fn from_file(self, asm_file: PathBuf) -> Self { if asm_file.extension().unwrap() == "asm" { self.from_asm_file(asm_file) @@ -303,6 +328,30 @@ impl Pipeline { } } + /// Reads a previously generated witness from the provided directory and + /// advances the pipeline to the `GeneratedWitness` stage. + pub fn read_generated_witness(mut self, directory: &Path) -> Self { + self.advance_to(Stage::OptimizedPil).unwrap(); + + let pil = match self.artifact.unwrap() { + Artifact::OptimzedPil(pil) => pil, + _ => panic!(), + }; + + let (fixed, degree_fixed) = read_poly_set::(&pil, directory); + let (witness, degree_witness) = read_poly_set::(&pil, directory); + assert_eq!(degree_fixed, degree_witness); + + Pipeline { + artifact: Some(Artifact::GeneratedWitness(GeneratedWitness { + pil, + constants: fixed, + witness: Some(witness), + })), + ..self + } + } + fn name_from_path(path: &Path) -> String { path.file_stem().unwrap().to_str().unwrap().to_string() } @@ -429,10 +478,20 @@ impl Pipeline { let backend = self .arguments .backend - .take() .expect("backend must be set before calling proving!"); let factory = backend.factory::(); - let backend = factory.create(pil.degree()); + let backend = if let Some(path) = self.arguments.setup_file.as_ref() { + let mut file = fs::File::open(path).unwrap(); + factory.create_from_setup(&mut file).unwrap() + } else { + factory.create(pil.degree()) + }; + + let existing_proof = self.arguments.existing_proof_file.as_ref().map(|path| { + let mut buf = Vec::new(); + fs::File::open(path).unwrap().read_to_end(&mut buf).unwrap(); + buf + }); // Even if we don't have all constants and witnesses, some backends will // still output the constraint serialization. @@ -440,7 +499,7 @@ impl Pipeline { &pil, &constants, witness.as_deref().unwrap_or_default(), - None, + existing_proof, ); let proof_result = ProofResult { @@ -544,7 +603,12 @@ impl Pipeline { if let Some(proof) = &proof_result.proof { // No need to bufferize the writing, because we write the whole // proof in one call. - let to_write = output_dir.join("proof.bin"); + let fname = if self.arguments.existing_proof_file.is_some() { + "proof_aggr.bin" + } else { + "proof.bin" + }; + let to_write = output_dir.join(fname); let mut proof_file = fs::File::create(&to_write).unwrap(); proof_file.write_all(proof).unwrap(); log::info!("Wrote {}.", to_write.display()); diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index 1b77dbe9f..a28f902c0 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -2,10 +2,9 @@ mod util; -use backend::{Backend, BackendType, Proof}; +use backend::{Backend, BackendType}; use clap::{CommandFactory, Parser, Subcommand}; use compiler::pipeline::{Pipeline, Stage}; -use compiler::util::{read_poly_set, FixedPolySet, WitnessPolySet}; use env_logger::fmt::Color; use env_logger::{Builder, Target}; use log::LevelFilter; @@ -14,7 +13,7 @@ use number::{Bn254Field, FieldElement, GoldilocksField}; use riscv::continuations::{rust_continuations, rust_continuations_dry_run}; use riscv::{compile_riscv_asm, compile_rust}; use std::collections::HashMap; -use std::io::{self, BufReader, BufWriter, Read}; +use std::io::{self, BufReader, BufWriter}; use std::path::PathBuf; use std::{borrow::Cow, fs, io::Write, path::Path}; use strum::{Display, EnumString, EnumVariantNames}; @@ -703,37 +702,14 @@ fn read_and_prove( proof_path: Option, params: Option, ) { - let pil = Pipeline::default() + Pipeline::::default() .from_file(file.to_path_buf()) - .optimized_pil() + .read_generated_witness(dir) + .with_setup_file(params.map(PathBuf::from)) + .with_existing_proof_file(proof_path.map(PathBuf::from)) + .with_backend(*backend_type) + .proof() .unwrap(); - - let fixed = read_poly_set::(&pil, dir); - let witness = read_poly_set::(&pil, dir); - - assert_eq!(fixed.1, witness.1); - - // TODO: Pull this into pipeline - let builder = backend_type.factory::(); - let backend = if let Some(filename) = params { - let mut file = fs::File::open(filename).unwrap(); - builder.create_from_setup(&mut file).unwrap() - } else { - builder.create(fixed.1) - }; - - let proof = proof_path.map(|filename| { - let mut buf = Vec::new(); - fs::File::open(filename) - .unwrap() - .read_to_end(&mut buf) - .unwrap(); - buf - }); - let is_aggr = proof.is_some(); - - let (proof, _) = backend.prove(&pil, &fixed.0, &witness.0, proof); - write_proving_results_to_fs(is_aggr, &proof, dir); } #[allow(clippy::print_stdout)] @@ -747,26 +723,6 @@ fn optimize_and_output(file: &str) { ); } -fn write_proving_results_to_fs(is_aggregation: bool, proof: &Option, output_dir: &Path) { - match proof { - Some(proof) => { - let fname = if is_aggregation { - "proof_aggr.bin" - } else { - "proof.bin" - }; - - // No need to bufferize the writing, because we write the whole - // proof in one call. - let to_write = output_dir.join(fname); - let mut proof_file = fs::File::create(&to_write).unwrap(); - proof_file.write_all(proof).unwrap(); - log::info!("Wrote {}.", to_write.display()); - } - None => log::warn!("No proof was generated"), - } -} - #[cfg(test)] mod test { use crate::{run_command, Commands, CsvRenderModeCLI, FieldArgument}; From 41a28a75f724ea9e808d25465d03966d6af84860 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 15:29:46 +0100 Subject: [PATCH 04/13] External Witness Values as String --- compiler/src/pipeline.rs | 7 ++----- compiler/src/test_util.rs | 6 +++--- compiler/tests/asm.rs | 2 +- compiler/tests/pil.rs | 12 ++++++------ powdr_cli/src/main.rs | 4 ---- 5 files changed, 12 insertions(+), 19 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index 52abba2a5..4be95b15f 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -210,14 +210,11 @@ impl Pipeline { pub fn with_external_witness_values( self, - external_witness_values: Vec<(&str, Vec)>, + external_witness_values: Vec<(String, Vec)>, ) -> Self { Pipeline { arguments: Arguments { - external_witness_values: external_witness_values - .into_iter() - .map(|(name, values)| (name.to_string(), values)) - .collect(), + external_witness_values, ..self.arguments }, ..self diff --git a/compiler/src/test_util.rs b/compiler/src/test_util.rs index d4152cf29..9506741ac 100644 --- a/compiler/src/test_util.rs +++ b/compiler/src/test_util.rs @@ -16,7 +16,7 @@ pub fn resolve_test_file(file_name: &str) -> PathBuf { pub fn verify_test_file( file_name: &str, inputs: Vec, - external_witness_values: Vec<(&str, Vec)>, + external_witness_values: Vec<(String, Vec)>, ) { let pipeline = Pipeline::default().from_file(resolve_test_file(file_name)); verify_pipeline(pipeline, inputs, external_witness_values) @@ -26,7 +26,7 @@ pub fn verify_asm_string( file_name: &str, contents: &str, inputs: Vec, - external_witness_values: Vec<(&str, Vec)>, + external_witness_values: Vec<(String, Vec)>, ) { let pipeline = Pipeline::default().from_asm_string(contents.to_string(), Some(PathBuf::from(file_name))); @@ -36,7 +36,7 @@ pub fn verify_asm_string( pub fn verify_pipeline( pipeline: Pipeline, inputs: Vec, - external_witness_values: Vec<(&str, Vec)>, + external_witness_values: Vec<(String, Vec)>, ) { let mut pipeline = pipeline .with_tmp_output() diff --git a/compiler/tests/asm.rs b/compiler/tests/asm.rs index e5e1e350f..f9be435f0 100644 --- a/compiler/tests/asm.rs +++ b/compiler/tests/asm.rs @@ -42,7 +42,7 @@ fn mem_write_once_external_write() { mem[17] = GoldilocksField::from(42); mem[62] = GoldilocksField::from(123); mem[255] = GoldilocksField::from(-1); - verify_test_file::(f, vec![], vec![("main.v", mem)]); + verify_test_file::(f, vec![], vec![("main.v".to_string(), mem)]); } #[test] diff --git a/compiler/tests/pil.rs b/compiler/tests/pil.rs index b91f8ed36..19da9f83d 100644 --- a/compiler/tests/pil.rs +++ b/compiler/tests/pil.rs @@ -41,14 +41,14 @@ fn test_external_witgen_fails_if_none_provided() { #[test] fn test_external_witgen_a_provided() { let f = "pil/external_witgen.pil"; - let external_witness = vec![("main.a", vec![GoldilocksField::from(3); 16])]; + let external_witness = vec![("main.a".to_string(), vec![GoldilocksField::from(3); 16])]; verify_test_file(f, vec![], external_witness); } #[test] fn test_external_witgen_b_provided() { let f = "pil/external_witgen.pil"; - let external_witness = vec![("main.b", vec![GoldilocksField::from(4); 16])]; + let external_witness = vec![("main.b".to_string(), vec![GoldilocksField::from(4); 16])]; verify_test_file(f, vec![], external_witness); } @@ -56,8 +56,8 @@ fn test_external_witgen_b_provided() { fn test_external_witgen_both_provided() { let f = "pil/external_witgen.pil"; let external_witness = vec![ - ("main.a", vec![GoldilocksField::from(3); 16]), - ("main.b", vec![GoldilocksField::from(4); 16]), + ("main.a".to_string(), vec![GoldilocksField::from(3); 16]), + ("main.b".to_string(), vec![GoldilocksField::from(4); 16]), ]; verify_test_file(f, vec![], external_witness); } @@ -67,9 +67,9 @@ fn test_external_witgen_both_provided() { fn test_external_witgen_fails_on_conflicting_external_witness() { let f = "pil/external_witgen.pil"; let external_witness = vec![ - ("main.a", vec![GoldilocksField::from(3); 16]), + ("main.a".to_string(), vec![GoldilocksField::from(3); 16]), // Does not satisfy b = a + 1 - ("main.b", vec![GoldilocksField::from(3); 16]), + ("main.b".to_string(), vec![GoldilocksField::from(3); 16]), ]; verify_test_file(f, vec![], external_witness); } diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index a28f902c0..d4d21bbf1 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -670,10 +670,6 @@ fn compile_with_csv_export( }) .unwrap_or(vec![]); - // Convert Vec<(String, Vec)> to Vec<(&str, Vec)> - let (strings, values): (Vec<_>, Vec<_>) = external_witness_values.into_iter().unzip(); - let external_witness_values = strings.iter().map(AsRef::as_ref).zip(values).collect(); - let output_dir = Path::new(&output_directory); let csv_mode = match csv_mode { From 376019c7ebb00a8941f510b73bbc0f3cb36c57f8 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 16:45:29 +0100 Subject: [PATCH 05/13] Refactor --- compiler/src/pipeline.rs | 8 + powdr_cli/src/main.rs | 317 +++++++++++++++++++------------------ riscv/src/continuations.rs | 5 +- 3 files changed, 173 insertions(+), 157 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index 4be95b15f..cd2ec8a98 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -641,6 +641,14 @@ impl Pipeline { Ok(()) } + pub fn asm_string(mut self) -> Result> { + self.advance_to(Stage::AsmString)?; + match self.artifact.unwrap() { + Artifact::AsmString(_, asm_string) => Ok(asm_string), + _ => panic!(), + } + } + pub fn analyzed_asm(mut self) -> Result, Vec> { self.advance_to(Stage::AnalyzedAsm)?; let Artifact::AnalyzedAsm(analyzed_asm) = self.artifact.unwrap() else { diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index d4d21bbf1..b4f4c9672 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -18,6 +18,35 @@ use std::path::PathBuf; use std::{borrow::Cow, fs, io::Write, path::Path}; use strum::{Display, EnumString, EnumVariantNames}; +fn add_external_witness_values( + pipeline: Pipeline, + witness_values: Option, +) -> Pipeline { + let external_witness_values = witness_values + .map(|csv_path| { + let csv_file = fs::File::open(csv_path).unwrap(); + let mut csv_writer = BufReader::new(&csv_file); + read_polys_csv_file::(&mut csv_writer) + }) + .unwrap_or(vec![]); + + pipeline.with_external_witness_values(external_witness_values) +} + +fn add_csv_settings( + pipeline: Pipeline, + export_csv: bool, + csv_mode: CsvRenderModeCLI, +) -> Pipeline { + let csv_mode = match csv_mode { + CsvRenderModeCLI::SignedBase10 => CsvRenderMode::SignedBase10, + CsvRenderModeCLI::UnsignedBase10 => CsvRenderMode::UnsignedBase10, + CsvRenderModeCLI::Hex => CsvRenderMode::Hex, + }; + + pipeline.with_witness_csv_settings(export_csv, csv_mode) +} + #[derive(Clone, EnumString, EnumVariantNames, Display)] pub enum FieldArgument { #[strum(serialize = "gl")] @@ -26,7 +55,7 @@ pub enum FieldArgument { Bn254, } -#[derive(Clone, EnumString, EnumVariantNames, Display)] +#[derive(Clone, Copy, EnumString, EnumVariantNames, Display)] pub enum CsvRenderModeCLI { #[strum(serialize = "i")] SignedBase10, @@ -420,69 +449,20 @@ fn run_command(command: Commands) { csv_mode, just_execute, continuations, - } => match (just_execute, continuations) { - (true, true) => { - assert!(matches!(field, FieldArgument::Gl)); - let inputs = split_inputs::(&inputs); - rust_continuations_dry_run( - Pipeline::default().from_asm_file(PathBuf::from(file)), - inputs, - ); - } - (true, false) => { - let contents = fs::read_to_string(&file).unwrap(); - let inputs = split_inputs::(&inputs); - let inputs: HashMap> = - vec![(GoldilocksField::from(0), inputs)] - .into_iter() - .collect(); - riscv_executor::execute::( - &contents, - &inputs, - &[], - riscv_executor::ExecMode::Fast, - ); - } - (false, true) => { - assert!(matches!(field, FieldArgument::Gl)); - let inputs = split_inputs::(&inputs); - let pipeline_factory = || { - Pipeline::default() - .from_asm_file(PathBuf::from(&file)) - .with_prover_inputs(vec![]) - }; - let pipeline_callback = - |mut pipeline: Pipeline| -> Result<(), Vec> { - pipeline.advance_to(Stage::GeneratedWitness)?; - if let Some(backend) = prove_with { - pipeline.with_backend(backend).proof()?; - } - Ok(()) - }; - - rust_continuations(pipeline_factory, pipeline_callback, inputs.clone()).unwrap(); - } - (false, false) => { - match call_with_field!(compile_with_csv_export::( - file, - output_directory, - witness_values, - inputs, - force, - prove_with, - export_csv, - csv_mode - )) { - Ok(()) => {} - Err(errors) => { - eprintln!("Errors:"); - for e in errors { - eprintln!("{e}"); - } - } - }; - } - }, + } => { + call_with_field!(run_pil::( + file, + output_directory, + witness_values, + inputs, + force, + prove_with, + export_csv, + csv_mode, + just_execute, + continuations + )); + } Commands::Prove { file, dir, @@ -541,12 +521,25 @@ fn run_rust( ) .ok_or_else(|| vec!["could not compile rust".to_string()])?; - handle_riscv_asm( - asm_file_path.to_str().unwrap(), - &asm_contents, - inputs, - output_dir, + let pipeline_factory = || { + Pipeline::::default().from_asm_string( + asm_contents.clone(), + Some(PathBuf::from(asm_file_path.to_str().unwrap())), + ) + }; + + let pipeline_factory = make_pipeline_factory( + pipeline_factory, + inputs.clone(), + output_dir.to_path_buf(), force_overwrite, + None, // witness_values, + false, // export_csv, + CsvRenderModeCLI::Hex, // csv_mode, + ); + run( + pipeline_factory, + inputs, prove_with, just_execute, continuations, @@ -576,12 +569,25 @@ fn run_riscv_asm( ) .ok_or_else(|| vec!["could not compile RISC-V assembly".to_string()])?; - handle_riscv_asm( - asm_file_path.to_str().unwrap(), - &asm_contents, - inputs, - output_dir, + let pipeline_factory = || { + Pipeline::::default().from_asm_string( + asm_contents.clone(), + Some(PathBuf::from(asm_file_path.to_str().unwrap())), + ) + }; + + let pipeline_factory = make_pipeline_factory( + pipeline_factory, + inputs.clone(), + output_dir.to_path_buf(), force_overwrite, + None, // witness_values, + false, // export_csv, + CsvRenderModeCLI::Hex, // csv_mode, + ); + run( + pipeline_factory, + inputs, prove_with, just_execute, continuations, @@ -590,69 +596,27 @@ fn run_riscv_asm( } #[allow(clippy::too_many_arguments)] -fn handle_riscv_asm( - file_name: &str, - contents: &str, +fn make_pipeline_factory( + pipeline_factory: impl Fn() -> Pipeline, inputs: Vec, - output_dir: &Path, + output_dir: PathBuf, force_overwrite: bool, - prove_with: Option, - just_execute: bool, - continuations: bool, -) -> Result<(), Vec> { - match (just_execute, continuations) { - (true, true) => { - rust_continuations_dry_run( - Pipeline::default() - .from_asm_string(contents.to_string(), Some(PathBuf::from(file_name))), - inputs, - ); - } - (true, false) => { - let mut inputs_hash: HashMap> = HashMap::default(); - inputs_hash.insert(0u32.into(), inputs); - riscv_executor::execute::( - contents, - &inputs_hash, - &[], - riscv_executor::ExecMode::Fast, - ); - } - (false, true) => { - let pipeline_factory = || { - Pipeline::default() - .with_output(output_dir.to_path_buf(), force_overwrite) - .from_asm_string(contents.to_string(), Some(PathBuf::from(file_name))) - .with_prover_inputs(inputs.clone()) - }; - let pipeline_callback = |mut pipeline: Pipeline| -> Result<(), Vec> { - pipeline.advance_to(Stage::GeneratedWitness)?; - if let Some(backend) = prove_with { - pipeline.with_backend(backend).proof()?; - } - Ok(()) - }; + witness_values: Option, + export_csv: bool, + csv_mode: CsvRenderModeCLI, +) -> impl Fn() -> Pipeline { + move || { + let pipeline = pipeline_factory() + .with_output(output_dir.clone(), force_overwrite) + .with_prover_inputs(inputs.clone()); - rust_continuations(pipeline_factory, pipeline_callback, inputs.clone())?; - } - (false, false) => { - let mut pipeline = Pipeline::default() - .with_output(output_dir.to_path_buf(), force_overwrite) - .from_asm_string(contents.to_string(), Some(PathBuf::from(file_name))) - .with_prover_inputs(inputs) - .with_backend(BackendType::PilStarkCli); - pipeline.advance_to(Stage::GeneratedWitness).unwrap(); - if let Some(backend) = prove_with { - pipeline = pipeline.with_backend(backend); - pipeline.proof().unwrap(); - } - } + let pipeline = add_external_witness_values(pipeline, witness_values.clone()); + add_csv_settings(pipeline, export_csv, csv_mode) } - Ok(()) } #[allow(clippy::too_many_arguments)] -fn compile_with_csv_export( +fn run_pil( file: String, output_directory: String, witness_values: Option, @@ -661,33 +625,80 @@ fn compile_with_csv_export( prove_with: Option, export_csv: bool, csv_mode: CsvRenderModeCLI, + just_execute: bool, + continuations: bool, +) { + let pipeline_factory = || Pipeline::::default().from_asm_file(PathBuf::from(&file)); + let inputs = split_inputs::(&inputs); + + let pipeline_factory = make_pipeline_factory( + pipeline_factory, + inputs.clone(), + PathBuf::from(output_directory), + force, + witness_values, + export_csv, + csv_mode, + ); + if let Err(errors) = run( + pipeline_factory, + inputs, + prove_with, + just_execute, + continuations, + ) { + eprintln!("Errors:"); + for e in errors { + eprintln!("{e}"); + } + }; +} + +#[allow(clippy::too_many_arguments)] +fn run( + pipeline_factory: impl Fn() -> Pipeline, + inputs: Vec, + prove_with: Option, + just_execute: bool, + continuations: bool, ) -> Result<(), Vec> { - let external_witness_values = witness_values - .map(|csv_path| { - let csv_file = fs::File::open(csv_path).unwrap(); - let mut csv_writer = BufReader::new(&csv_file); - read_polys_csv_file::(&mut csv_writer) - }) - .unwrap_or(vec![]); - - let output_dir = Path::new(&output_directory); - - let csv_mode = match csv_mode { - CsvRenderModeCLI::SignedBase10 => CsvRenderMode::SignedBase10, - CsvRenderModeCLI::UnsignedBase10 => CsvRenderMode::UnsignedBase10, - CsvRenderModeCLI::Hex => CsvRenderMode::Hex, + let bootloader_inputs = if continuations { + rust_continuations_dry_run(pipeline_factory(), inputs.clone()) + } else { + vec![] }; - let mut pipeline = Pipeline::default() - .with_output(output_dir.to_path_buf(), force) - .from_file(PathBuf::from(file)) - .with_external_witness_values(external_witness_values) - .with_witness_csv_settings(export_csv, csv_mode) - .with_prover_inputs(split_inputs(&inputs)); - - pipeline.advance_to(Stage::GeneratedWitness).unwrap(); - prove_with.map(|backend| pipeline.with_backend(backend).proof().unwrap()); + match (just_execute, continuations) { + (true, true) => { + // Nothing to do + } + (true, false) => { + let mut inputs_hash: HashMap> = HashMap::default(); + inputs_hash.insert(0u32.into(), inputs); + riscv_executor::execute::( + &pipeline_factory().asm_string().unwrap(), + &inputs_hash, + &[], + riscv_executor::ExecMode::Fast, + ); + } + (false, true) => { + let pipeline_callback = |mut pipeline: Pipeline| -> Result<(), Vec> { + pipeline.advance_to(Stage::GeneratedWitness)?; + if let Some(backend) = prove_with { + pipeline.with_backend(backend).proof()?; + } + Ok(()) + }; + rust_continuations(pipeline_factory, pipeline_callback, bootloader_inputs)?; + } + (false, false) => { + let mut pipeline = pipeline_factory(); + pipeline.advance_to(Stage::GeneratedWitness).unwrap(); + prove_with.map(|backend| pipeline.with_backend(backend).proof().unwrap()); + } + } Ok(()) } diff --git a/riscv/src/continuations.rs b/riscv/src/continuations.rs index 08d46fdae..52176a50e 100644 --- a/riscv/src/continuations.rs +++ b/riscv/src/continuations.rs @@ -49,15 +49,12 @@ fn transposed_trace(trace: &ExecutionTrace) -> HashMap( pipeline_factory: PipelineFactory, pipeline_callback: PipelineCallback, - inputs: Vec, + bootloader_inputs: Vec>, ) -> Result<(), E> where PipelineFactory: Fn() -> Pipeline, PipelineCallback: Fn(Pipeline) -> Result<(), E>, { - log::info!("Dry running execution to collect bootloader inputs..."); - let pipeline = pipeline_factory(); - let bootloader_inputs = rust_continuations_dry_run(pipeline, inputs.clone()); let num_chunks = bootloader_inputs.len(); bootloader_inputs From 7e1793475dbd4d60c7db3a32428e2be6c3da4047 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 17:06:41 +0100 Subject: [PATCH 06/13] Refactor --- powdr_cli/src/main.rs | 60 +++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 33 deletions(-) diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index b4f4c9672..a585c862b 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -352,7 +352,7 @@ fn main() -> Result<(), io::Error> { #[allow(clippy::print_stderr)] fn run_command(command: Commands) { - match command { + let result = match command { Commands::Rust { file, field, @@ -370,7 +370,7 @@ fn run_command(command: Commands) { } None => riscv::CoProcessors::base(), }; - if let Err(errors) = call_with_field!(run_rust::( + call_with_field!(run_rust::( &file, split_inputs(&inputs), Path::new(&output_directory), @@ -379,12 +379,7 @@ fn run_command(command: Commands) { coprocessors, just_execute, continuations - )) { - eprintln!("Errors:"); - for e in errors { - eprintln!("{e}"); - } - }; + )) } Commands::RiscvAsm { files, @@ -410,7 +405,7 @@ fn run_command(command: Commands) { } None => riscv::CoProcessors::base(), }; - if let Err(errors) = call_with_field!(run_riscv_asm::( + call_with_field!(run_riscv_asm::( &name, files.into_iter(), split_inputs(&inputs), @@ -420,22 +415,19 @@ fn run_command(command: Commands) { coprocessors, just_execute, continuations - )) { - eprintln!("Errors:"); - for e in errors { - eprintln!("{e}"); - } - }; + )) } Commands::Reformat { file } => { let contents = fs::read_to_string(&file).unwrap(); match parser::parse::(Some(&file), &contents) { Ok(ast) => println!("{ast}"), Err(err) => err.output_to_stderr(), - } + }; + Ok(()) } Commands::OptimizePIL { file, field } => { - call_with_field!(optimize_and_output::(&file)) + call_with_field!(optimize_and_output::(&file)); + Ok(()) } Commands::Pil { file, @@ -461,7 +453,7 @@ fn run_command(command: Commands) { csv_mode, just_execute, continuations - )); + )) } Commands::Prove { file, @@ -473,7 +465,7 @@ fn run_command(command: Commands) { } => { let pil = Path::new(&file); let dir = Path::new(&dir); - call_with_field!(read_and_prove::(pil, dir, &backend, proof, params)); + call_with_field!(read_and_prove::(pil, dir, &backend, proof, params)) } Commands::Setup { size, @@ -482,8 +474,15 @@ fn run_command(command: Commands) { backend, } => { call_with_field!(setup::(size, dir, backend)); + Ok(()) } }; + if let Err(errors) = result { + for error in errors { + eprintln!("{}", error); + } + std::process::exit(1); + } } fn setup(size: u64, dir: String, backend_type: BackendType) { @@ -627,12 +626,11 @@ fn run_pil( csv_mode: CsvRenderModeCLI, just_execute: bool, continuations: bool, -) { - let pipeline_factory = || Pipeline::::default().from_asm_file(PathBuf::from(&file)); +) -> Result<(), Vec> { let inputs = split_inputs::(&inputs); let pipeline_factory = make_pipeline_factory( - pipeline_factory, + || Pipeline::::default().from_asm_file(PathBuf::from(&file)), inputs.clone(), PathBuf::from(output_directory), force, @@ -640,18 +638,14 @@ fn run_pil( export_csv, csv_mode, ); - if let Err(errors) = run( + run( pipeline_factory, inputs, prove_with, just_execute, continuations, - ) { - eprintln!("Errors:"); - for e in errors { - eprintln!("{e}"); - } - }; + )?; + Ok(()) } #[allow(clippy::too_many_arguments)] @@ -670,7 +664,7 @@ fn run( match (just_execute, continuations) { (true, true) => { - // Nothing to do + // Already ran when computing bootloader inputs, nothing else to do. } (true, false) => { let mut inputs_hash: HashMap> = HashMap::default(); @@ -708,15 +702,15 @@ fn read_and_prove( backend_type: &BackendType, proof_path: Option, params: Option, -) { +) -> Result<(), Vec> { Pipeline::::default() .from_file(file.to_path_buf()) .read_generated_witness(dir) .with_setup_file(params.map(PathBuf::from)) .with_existing_proof_file(proof_path.map(PathBuf::from)) .with_backend(*backend_type) - .proof() - .unwrap(); + .proof()?; + Ok(()) } #[allow(clippy::print_stdout)] From 8e6ec62fb5471e39d5c40322e39e4266979c80a8 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 17:15:32 +0100 Subject: [PATCH 07/13] Refactor --- powdr_cli/src/main.rs | 58 ++++++++++++++++--------------------------- 1 file changed, 21 insertions(+), 37 deletions(-) diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index a585c862b..ef0fee348 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -18,33 +18,37 @@ use std::path::PathBuf; use std::{borrow::Cow, fs, io::Write, path::Path}; use strum::{Display, EnumString, EnumVariantNames}; -fn add_external_witness_values( - pipeline: Pipeline, +#[allow(clippy::too_many_arguments)] +fn bind_cli_args( + pipeline_factory: impl Fn() -> Pipeline, + inputs: Vec, + output_dir: PathBuf, + force_overwrite: bool, witness_values: Option, -) -> Pipeline { - let external_witness_values = witness_values + export_csv: bool, + csv_mode: CsvRenderModeCLI, +) -> impl Fn() -> Pipeline { + let witness_values = witness_values .map(|csv_path| { let csv_file = fs::File::open(csv_path).unwrap(); let mut csv_writer = BufReader::new(&csv_file); - read_polys_csv_file::(&mut csv_writer) + read_polys_csv_file::(&mut csv_writer) }) .unwrap_or(vec![]); - pipeline.with_external_witness_values(external_witness_values) -} - -fn add_csv_settings( - pipeline: Pipeline, - export_csv: bool, - csv_mode: CsvRenderModeCLI, -) -> Pipeline { let csv_mode = match csv_mode { CsvRenderModeCLI::SignedBase10 => CsvRenderMode::SignedBase10, CsvRenderModeCLI::UnsignedBase10 => CsvRenderMode::UnsignedBase10, CsvRenderModeCLI::Hex => CsvRenderMode::Hex, }; - pipeline.with_witness_csv_settings(export_csv, csv_mode) + move || { + pipeline_factory() + .with_output(output_dir.clone(), force_overwrite) + .with_external_witness_values(witness_values.clone()) + .with_witness_csv_settings(export_csv, csv_mode) + .with_prover_inputs(inputs.clone()) + } } #[derive(Clone, EnumString, EnumVariantNames, Display)] @@ -527,7 +531,7 @@ fn run_rust( ) }; - let pipeline_factory = make_pipeline_factory( + let pipeline_factory = bind_cli_args( pipeline_factory, inputs.clone(), output_dir.to_path_buf(), @@ -575,7 +579,7 @@ fn run_riscv_asm( ) }; - let pipeline_factory = make_pipeline_factory( + let pipeline_factory = bind_cli_args( pipeline_factory, inputs.clone(), output_dir.to_path_buf(), @@ -594,26 +598,6 @@ fn run_riscv_asm( Ok(()) } -#[allow(clippy::too_many_arguments)] -fn make_pipeline_factory( - pipeline_factory: impl Fn() -> Pipeline, - inputs: Vec, - output_dir: PathBuf, - force_overwrite: bool, - witness_values: Option, - export_csv: bool, - csv_mode: CsvRenderModeCLI, -) -> impl Fn() -> Pipeline { - move || { - let pipeline = pipeline_factory() - .with_output(output_dir.clone(), force_overwrite) - .with_prover_inputs(inputs.clone()); - - let pipeline = add_external_witness_values(pipeline, witness_values.clone()); - add_csv_settings(pipeline, export_csv, csv_mode) - } -} - #[allow(clippy::too_many_arguments)] fn run_pil( file: String, @@ -629,7 +613,7 @@ fn run_pil( ) -> Result<(), Vec> { let inputs = split_inputs::(&inputs); - let pipeline_factory = make_pipeline_factory( + let pipeline_factory = bind_cli_args( || Pipeline::::default().from_asm_file(PathBuf::from(&file)), inputs.clone(), PathBuf::from(output_directory), From 726b414e4e82c40cf70cd797eb3269b8a294e451 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 17:19:13 +0100 Subject: [PATCH 08/13] Test now working? --- riscv/tests/riscv.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/riscv/tests/riscv.rs b/riscv/tests/riscv.rs index 537873cc9..909f97752 100644 --- a/riscv/tests/riscv.rs +++ b/riscv/tests/riscv.rs @@ -165,7 +165,6 @@ fn test_many_chunks_dry() { #[test] #[ignore = "Too slow"] -#[should_panic(expected = "Verified did not say 'PIL OK'.")] fn test_many_chunks() { // Compiles and runs the many_chunks.rs example with continuations, runs the full // witness generation & verifies it using Pilcom. From d31c82bcfb1b58a1a20bebed93eef635ff42fb45 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 17:22:32 +0100 Subject: [PATCH 09/13] No, test not working --- riscv/tests/riscv.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/riscv/tests/riscv.rs b/riscv/tests/riscv.rs index 909f97752..a7e5b6875 100644 --- a/riscv/tests/riscv.rs +++ b/riscv/tests/riscv.rs @@ -165,6 +165,7 @@ fn test_many_chunks_dry() { #[test] #[ignore = "Too slow"] +#[should_panic(expected = "Verified did not say 'PIL OK'.")] fn test_many_chunks() { // Compiles and runs the many_chunks.rs example with continuations, runs the full // witness generation & verifies it using Pilcom. @@ -185,7 +186,8 @@ fn test_many_chunks() { verify_pipeline(pipeline, vec![], vec![]); Ok(()) }; - rust_continuations(pipeline_factory, pipeline_callback, vec![]).unwrap(); + let bootloader_inputs = rust_continuations_dry_run(pipeline_factory(), vec![]); + rust_continuations(pipeline_factory, pipeline_callback, bootloader_inputs).unwrap(); } fn verify_file(case: &str, inputs: Vec, coprocessors: &CoProcessors) { From c69f1b3bdcb921b17d6cd409557642208cb19f5b Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 17:31:08 +0100 Subject: [PATCH 10/13] CSV export for Rust code as well --- powdr_cli/src/main.rs | 46 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index ef0fee348..2b620b9c4 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -173,6 +173,17 @@ enum Commands { #[arg(value_parser = clap_enum_variants!(BackendType))] prove_with: Option, + /// Generate a CSV file containing the fixed and witness column values. Useful for debugging purposes. + #[arg(long)] + #[arg(default_value_t = false)] + export_csv: bool, + + /// How to render field elements in the csv file + #[arg(long)] + #[arg(default_value_t = CsvRenderModeCLI::Hex)] + #[arg(value_parser = clap_enum_variants!(CsvRenderModeCLI))] + csv_mode: CsvRenderModeCLI, + /// Comma-separated list of coprocessors. #[arg(long)] coprocessors: Option, @@ -221,6 +232,17 @@ enum Commands { #[arg(value_parser = clap_enum_variants!(BackendType))] prove_with: Option, + /// Generate a CSV file containing the fixed and witness column values. Useful for debugging purposes. + #[arg(long)] + #[arg(default_value_t = false)] + export_csv: bool, + + /// How to render field elements in the csv file + #[arg(long)] + #[arg(default_value_t = CsvRenderModeCLI::Hex)] + #[arg(value_parser = clap_enum_variants!(CsvRenderModeCLI))] + csv_mode: CsvRenderModeCLI, + /// Comma-separated list of coprocessors. #[arg(long)] coprocessors: Option, @@ -364,6 +386,8 @@ fn run_command(command: Commands) { output_directory, force, prove_with, + export_csv, + csv_mode, coprocessors, just_execute, continuations, @@ -380,6 +404,8 @@ fn run_command(command: Commands) { Path::new(&output_directory), force, prove_with, + export_csv, + csv_mode, coprocessors, just_execute, continuations @@ -392,6 +418,8 @@ fn run_command(command: Commands) { output_directory, force, prove_with, + export_csv, + csv_mode, coprocessors, just_execute, continuations, @@ -416,6 +444,8 @@ fn run_command(command: Commands) { Path::new(&output_directory), force, prove_with, + export_csv, + csv_mode, coprocessors, just_execute, continuations @@ -511,6 +541,8 @@ fn run_rust( output_dir: &Path, force_overwrite: bool, prove_with: Option, + export_csv: bool, + csv_mode: CsvRenderModeCLI, coprocessors: riscv::CoProcessors, just_execute: bool, continuations: bool, @@ -536,9 +568,9 @@ fn run_rust( inputs.clone(), output_dir.to_path_buf(), force_overwrite, - None, // witness_values, - false, // export_csv, - CsvRenderModeCLI::Hex, // csv_mode, + None, + export_csv, + csv_mode, ); run( pipeline_factory, @@ -558,6 +590,8 @@ fn run_riscv_asm( output_dir: &Path, force_overwrite: bool, prove_with: Option, + export_csv: bool, + csv_mode: CsvRenderModeCLI, coprocessors: riscv::CoProcessors, just_execute: bool, continuations: bool, @@ -584,9 +618,9 @@ fn run_riscv_asm( inputs.clone(), output_dir.to_path_buf(), force_overwrite, - None, // witness_values, - false, // export_csv, - CsvRenderModeCLI::Hex, // csv_mode, + None, + export_csv, + csv_mode, ); run( pipeline_factory, From 23ee897878296609b79d34a749b85414df7b4127 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Mon, 11 Dec 2023 17:35:07 +0100 Subject: [PATCH 11/13] More concise argument calls --- compiler/src/pipeline.rs | 68 +++++++++++----------------------------- 1 file changed, 19 insertions(+), 49 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index cd2ec8a98..7862abf1e 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -209,78 +209,48 @@ impl Pipeline { } pub fn with_external_witness_values( - self, + mut self, external_witness_values: Vec<(String, Vec)>, ) -> Self { - Pipeline { - arguments: Arguments { - external_witness_values, - ..self.arguments - }, - ..self - } + self.arguments.external_witness_values = external_witness_values; + self } pub fn with_witness_csv_settings( - self, + mut self, export_witness_csv: bool, csv_render_mode: CsvRenderMode, ) -> Self { - Pipeline { - arguments: Arguments { - csv_render_mode, - export_witness_csv, - ..self.arguments - }, - ..self - } + self.arguments.export_witness_csv = export_witness_csv; + self.arguments.csv_render_mode = csv_render_mode; + self } - pub fn add_query_callback(self, query_callback: Box>) -> Self { + pub fn add_query_callback(mut self, query_callback: Box>) -> Self { let query_callback = match self.arguments.query_callback { Some(old_callback) => Box::new(chain_callbacks(old_callback, query_callback)), None => query_callback, }; - Pipeline { - arguments: Arguments { - query_callback: Some(query_callback), - ..self.arguments - }, - ..self - } + self.arguments.query_callback = Some(query_callback); + self } pub fn with_prover_inputs(self, inputs: Vec) -> Self { self.add_query_callback(Box::new(inputs_to_query_callback(inputs))) } - pub fn with_backend(self, backend: BackendType) -> Self { - Pipeline { - arguments: Arguments { - backend: Some(backend), - ..self.arguments - }, - ..self - } + pub fn with_backend(mut self, backend: BackendType) -> Self { + self.arguments.backend = Some(backend); + self } - pub fn with_setup_file(self, setup_file: Option) -> Self { - Pipeline { - arguments: Arguments { - setup_file, - ..self.arguments - }, - ..self - } + pub fn with_setup_file(mut self, setup_file: Option) -> Self { + self.arguments.setup_file = setup_file; + self } - pub fn with_existing_proof_file(self, existing_proof_file: Option) -> Self { - Pipeline { - arguments: Arguments { - existing_proof_file, - ..self.arguments - }, - ..self - } + pub fn with_existing_proof_file(mut self, existing_proof_file: Option) -> Self { + self.arguments.existing_proof_file = existing_proof_file; + self } pub fn from_file(self, asm_file: PathBuf) -> Self { From 05b6f154d8c012e07db618ef55a48b15f60d352d Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Tue, 12 Dec 2023 13:09:34 +0100 Subject: [PATCH 12/13] Small fixes --- powdr_cli/src/main.rs | 32 ++++++++++++++++++-------------- riscv/src/continuations.rs | 3 +++ 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/powdr_cli/src/main.rs b/powdr_cli/src/main.rs index 2b620b9c4..427eaacb3 100644 --- a/powdr_cli/src/main.rs +++ b/powdr_cli/src/main.rs @@ -18,6 +18,8 @@ use std::path::PathBuf; use std::{borrow::Cow, fs, io::Write, path::Path}; use strum::{Display, EnumString, EnumVariantNames}; +/// Transforms a pipeline factory into a pipeline factory that binds CLI arguments like +/// the output directory and the CSV export settings to the pipeline. #[allow(clippy::too_many_arguments)] fn bind_cli_args( pipeline_factory: impl Fn() -> Pipeline, @@ -43,6 +45,9 @@ fn bind_cli_args( }; move || { + // Note that we can't just take an existing pipeline here instead of + // a factory, as Pipeline doesn't currently implement Clone, so we need to + // create a new one each time. pipeline_factory() .with_output(output_dir.clone(), force_overwrite) .with_external_witness_values(witness_values.clone()) @@ -648,7 +653,7 @@ fn run_pil( let inputs = split_inputs::(&inputs); let pipeline_factory = bind_cli_args( - || Pipeline::::default().from_asm_file(PathBuf::from(&file)), + || Pipeline::::default().from_file(PathBuf::from(&file)), inputs.clone(), PathBuf::from(output_directory), force, @@ -666,7 +671,6 @@ fn run_pil( Ok(()) } -#[allow(clippy::too_many_arguments)] fn run( pipeline_factory: impl Fn() -> Pipeline, inputs: Vec, @@ -680,6 +684,12 @@ fn run( vec![] }; + let generate_witness_and_prove_maybe = |mut pipeline: Pipeline| -> Result<(), Vec> { + pipeline.advance_to(Stage::GeneratedWitness)?; + prove_with.map(|backend| pipeline.with_backend(backend).proof().unwrap()); + Ok(()) + }; + match (just_execute, continuations) { (true, true) => { // Already ran when computing bootloader inputs, nothing else to do. @@ -695,20 +705,14 @@ fn run( ); } (false, true) => { - let pipeline_callback = |mut pipeline: Pipeline| -> Result<(), Vec> { - pipeline.advance_to(Stage::GeneratedWitness)?; - if let Some(backend) = prove_with { - pipeline.with_backend(backend).proof()?; - } - Ok(()) - }; - - rust_continuations(pipeline_factory, pipeline_callback, bootloader_inputs)?; + rust_continuations( + pipeline_factory, + generate_witness_and_prove_maybe, + bootloader_inputs, + )?; } (false, false) => { - let mut pipeline = pipeline_factory(); - pipeline.advance_to(Stage::GeneratedWitness).unwrap(); - prove_with.map(|backend| pipeline.with_backend(backend).proof().unwrap()); + generate_witness_and_prove_maybe(pipeline_factory())?; } } Ok(()) diff --git a/riscv/src/continuations.rs b/riscv/src/continuations.rs index 52176a50e..c25c09bff 100644 --- a/riscv/src/continuations.rs +++ b/riscv/src/continuations.rs @@ -62,6 +62,9 @@ where .enumerate() .map(|(i, bootloader_inputs)| -> Result<(), E> { log::info!("Running chunk {} / {}...", i + 1, num_chunks); + // TODO(#840): If Pipeline implemented Clone, we could advance it once to + // the OptimizedPil stage and clone it here instead of creating and + // running a fresh pipeline for each chunk. let pipeline = pipeline_factory(); let pipeline = add_bootloader_inputs(pipeline, bootloader_inputs); pipeline_callback(pipeline)?; From 6698ce15a620c549ef1e5099c3f52dac21f6cd12 Mon Sep 17 00:00:00 2001 From: Georg Wiese Date: Tue, 12 Dec 2023 16:38:57 +0100 Subject: [PATCH 13/13] Refactor artifact writing --- compiler/src/pipeline.rs | 117 +++++++++++++++++++++------------------ 1 file changed, 64 insertions(+), 53 deletions(-) diff --git a/compiler/src/pipeline.rs b/compiler/src/pipeline.rs index 7862abf1e..ac5c5165b 100644 --- a/compiler/src/pipeline.rs +++ b/compiler/src/pipeline.rs @@ -102,14 +102,22 @@ enum Artifact { Proof(ProofResult), } +/// Optional Arguments for various stages of the pipeline. #[derive(Default)] struct Arguments { + /// Externally computed witness values for witness generation. external_witness_values: Vec<(String, Vec)>, + /// Callback for queries for witness generation. query_callback: Option>>, + /// Backend to use for proving. If None, proving will fail. backend: Option, + /// CSV render mode for witness generation. csv_render_mode: CsvRenderMode, + /// Whether to export the witness as a CSV file. export_witness_csv: bool, + /// The optional setup file to use for proving. setup_file: Option, + /// The optional existing proof file to use for aggregation. existing_proof_file: Option, } @@ -132,6 +140,7 @@ pub struct Pipeline { // Note that there is some redundancy with `output_dir`, but the Temp // object has to live for the lifetime of the pipeline, so we keep it here. tmp_dir: Option, + /// Optional arguments for various stages of the pipeline. arguments: Arguments, } @@ -185,7 +194,7 @@ where /// .with_backend(BackendType::PilStarkCli); /// /// // Advance to some stage (which might have side effects) -/// pipeline.advance_to(Stage::Proof).unwrap(); +/// pipeline.advance_to(Stage::OptimizedPil).unwrap(); /// /// // Get the result /// let proof = pipeline.proof().unwrap(); @@ -485,39 +494,46 @@ impl Pipeline { Ok(()) } + /// Returns the path to the output file if the output directory is set. + /// Fails if the file already exists and `force_overwrite` is false. + fn path_if_should_write String>( + &self, + file_name_from_pipeline_name: F, + ) -> Result, Vec> { + self.output_dir + .as_ref() + .map(|output_dir| { + let name = self + .name + .as_ref() + .expect("name must be set if output_dir is set"); + let path = output_dir.join(file_name_from_pipeline_name(name)); + if path.exists() && !self.force_overwrite { + Err(vec![format!( + "{} already exists! Use --force to overwrite.", + path.to_str().unwrap() + )])?; + } + log::info!("Writing {}.", path.to_str().unwrap()); + Ok(path) + }) + .transpose() + } + fn maybe_write_pil(&self, content: &C, suffix: &str) -> Result<(), Vec> { - if let Some(output_dir) = &self.output_dir { - let name = self - .name - .as_ref() - .expect("name must be set if output_dir is set"); - let output_file = output_dir.join(format!("{name}{suffix}.pil")); - if !output_file.exists() || self.force_overwrite { - fs::write(&output_file, format!("{content}")).map_err(|e| { - vec![format!( - "Error writing {}: {e}", - output_file.to_str().unwrap() - )] - })?; - self.log(&format!("Wrote {}.", output_file.to_str().unwrap())); - } else { - return Err(vec![format!( - "{} already exists! Use --force to overwrite.", - output_file.to_str().unwrap() - )]); - } + if let Some(path) = self.path_if_should_write(|name| format!("{name}{suffix}.pil"))? { + fs::write(&path, format!("{content}")) + .map_err(|e| vec![format!("Error writing {}: {e}", path.to_str().unwrap())])?; } Ok(()) } fn maybe_write_constants(&self, constants: &[(String, Vec)]) -> Result<(), Vec> { - if let Some(output_dir) = &self.output_dir { - let to_write = output_dir.join("constants.bin"); + if let Some(path) = self.path_if_should_write(|_| "constants.bin".to_string())? { write_polys_file( - &mut BufWriter::new(&mut fs::File::create(&to_write).unwrap()), + &mut BufWriter::new(&mut fs::File::create(path).unwrap()), constants, ); - log::info!("Wrote {}.", to_write.display()); } Ok(()) } @@ -527,17 +543,17 @@ impl Pipeline { fixed: &[(String, Vec)], witness: &Option)>>, ) -> Result<(), Vec> { - if let Some(output_dir) = &self.output_dir { - if let Some(witness) = witness.as_ref() { - let to_write = output_dir.join("commits.bin"); + if let Some(witness) = witness.as_ref() { + if let Some(path) = self.path_if_should_write(|_| "commits.bin".to_string())? { write_polys_file( - &mut BufWriter::new(&mut fs::File::create(&to_write).unwrap()), + &mut BufWriter::new(&mut fs::File::create(path).unwrap()), witness, ); - log::info!("Wrote {}.", to_write.display()); } + } - if self.arguments.export_witness_csv { + if self.arguments.export_witness_csv { + if let Some(path) = self.path_if_should_write(|_| "columns.csv".to_string())? { let columns = fixed .iter() .chain(match witness.as_ref() { @@ -546,41 +562,36 @@ impl Pipeline { }) .collect::>(); - let csv_path = Path::new(output_dir).join("columns.csv"); - let mut csv_file = - fs::File::create(&csv_path).map_err(|e| vec![format!("{}", e)])?; + let mut csv_file = fs::File::create(path).map_err(|e| vec![format!("{}", e)])?; let mut csv_writer = BufWriter::new(&mut csv_file); write_polys_csv_file(&mut csv_writer, self.arguments.csv_render_mode, &columns); - log::info!("Wrote {}.", csv_path.display()); } } + Ok(()) } fn maybe_wite_proof(&self, proof_result: &ProofResult) -> Result<(), Vec> { - if let Some(output_dir) = &self.output_dir { - if let Some(constraints_serialization) = &proof_result.constraints_serialization { - let to_write = output_dir.join("constraints.json"); - let mut file = fs::File::create(&to_write).unwrap(); + if let Some(constraints_serialization) = &proof_result.constraints_serialization { + if let Some(path) = self.path_if_should_write(|_| "constraints.json".to_string())? { + let mut file = fs::File::create(path).unwrap(); file.write_all(constraints_serialization.as_bytes()) .unwrap(); - log::info!("Wrote {}.", to_write.display()); - } - if let Some(proof) = &proof_result.proof { - // No need to bufferize the writing, because we write the whole - // proof in one call. - let fname = if self.arguments.existing_proof_file.is_some() { - "proof_aggr.bin" - } else { - "proof.bin" - }; - let to_write = output_dir.join(fname); - let mut proof_file = fs::File::create(&to_write).unwrap(); - proof_file.write_all(proof).unwrap(); - log::info!("Wrote {}.", to_write.display()); } } + if let Some(proof) = &proof_result.proof { + let fname = if self.arguments.existing_proof_file.is_some() { + "proof_aggr.bin" + } else { + "proof.bin" + }; + if let Some(path) = self.path_if_should_write(|_| fname.to_string())? { + let mut proof_file = fs::File::create(path).unwrap(); + proof_file.write_all(proof).unwrap(); + } + } + Ok(()) }