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.

Sign up for GitHub

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

Bug fix: Include intermediates from receives in machine parts #2546

Merged
merged 2 commits into from
Mar 14, 2025
Merged

Bug fix: Include intermediates from receives in machine parts #2546

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f50e99d
Bug fix: Include intermediates from receives in machine parts
georgwiese Mar 13, 2025
337083f
Update executor/src/witgen/machines/machine_extractor.rs
chriseth Mar 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions executor/src/witgen/machines/machine_extractor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::witgen::data_structures::identity::Identity;
use crate::witgen::machines::dynamic_machine::DynamicMachine;
use crate::witgen::machines::second_stage_machine::SecondStageMachine;
use crate::witgen::machines::{write_once_memory::WriteOnceMemory, MachineParts};
use powdr_ast::analyzed::AlgebraicExpression;

use powdr_ast::analyzed::{
self, AlgebraicExpression as Expression, PolyID, PolynomialReference, Reference,
Expand Down Expand Up @@ -136,11 +137,6 @@ impl<'a, T: FieldElement> MachineExtractor<'a, T> {

publics.add_all(machine_identities.as_slice()).unwrap();

let machine_intermediates = intermediates_in_identities(
&machine_identities,
&self.fixed.intermediate_definitions,
);

// Connections that call into the current machine
let machine_receives = self
.fixed
Expand All @@ -158,6 +154,14 @@ impl<'a, T: FieldElement> MachineExtractor<'a, T> {
.collect::<BTreeMap<_, _>>();
assert!(machine_receives.contains_key(&bus_receive.bus_id));

let machine_intermediates = intermediates_in_expressions(
machine_identities
.iter()
.flat_map(|i| i.all_children())
.chain(machine_receives.values().flat_map(|r| r.all_children())),
&self.fixed.intermediate_definitions,
);

let prover_functions = prover_functions
.iter()
.copied()
Expand Down Expand Up @@ -241,8 +245,10 @@ impl<'a, T: FieldElement> MachineExtractor<'a, T> {
.difference(&multiplicity_columns)
.cloned()
.collect::<HashSet<_>>();
let main_intermediates =
intermediates_in_identities(&base_identities, &self.fixed.intermediate_definitions);
let main_intermediates = intermediates_in_expressions(
base_identities.iter().flat_map(|i| i.all_children()),
&self.fixed.intermediate_definitions,
);

log::trace!(
"\nThe base machine contains the following witnesses:\n{}\n identities:\n{}\n and prover functions:\n{}",
Expand Down Expand Up @@ -488,15 +494,13 @@ fn try_as_intermediate_ref<T: FieldElement>(expr: &Expression<T>) -> Option<(Pol
}
}

/// Returns all intermediate columns referenced in the identities as a map to their name.
/// Returns all intermediate columns referenced in the expression as a map to their name.
/// Follows intermediate references recursively.
fn intermediates_in_identities<T: FieldElement>(
identities: &[&Identity<T>],
fn intermediates_in_expressions<'a, T: FieldElement>(
expressions: impl Iterator<Item = &'a AlgebraicExpression<T>>,
intermediate_definitions: &BTreeMap<AlgebraicReferenceThin, Expression<T>>,
) -> HashMap<PolyID, String> {
let mut queue = identities
.iter()
.flat_map(|id| id.all_children())
let mut queue = expressions
.filter_map(try_as_intermediate_ref)
.collect::<BTreeSet<_>>();
let mut intermediates = HashMap::new();
Expand Down
Loading