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

[fud2] update path finding to support multi state input and output #2134

Merged
merged 38 commits into from
Jul 8, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
59e63b3
generalize op type
jku20 Jun 7, 2024
2641b54
output used output states with operation
jku20 Jun 7, 2024
a3b9db2
generalize find_path for many inputs and outputs
jku20 Jun 10, 2024
66a3f15
cargo fmt
jku20 Jun 11, 2024
d9a3fb3
make op graph use adjacency lists and algo change
jku20 Jun 13, 2024
b1d83b8
fix find_path algorithm
jku20 Jun 18, 2024
7d789ab
remove debug print statements
jku20 Jun 18, 2024
edbafaf
remove extra parenthesis
jku20 Jun 18, 2024
d4b84ce
stop unreachable ops being considered for plans
jku20 Jun 19, 2024
75d6d85
making file naming not depend on stems and paths
jku20 Jun 19, 2024
1bf07a2
generalize emit logic
jku20 Jun 20, 2024
d36f79d
implement requests for multiple inputs and outputs
jku20 Jun 21, 2024
a035a94
cargo fmt
jku20 Jun 24, 2024
4090f6e
use simple enumerative search for find_path
jku20 Jun 25, 2024
252e898
cargo clippy
jku20 Jun 25, 2024
2967705
Merge branch 'main' into fud2-multi-input-output-find-path
jku20 Jun 25, 2024
7a5d63a
fix off by one
jku20 Jun 25, 2024
de0dbb5
modify cli to support multiple inputs and outputs
jku20 Jun 25, 2024
9fcfab7
fix name generation on cyclic plans
jku20 Jun 25, 2024
58b402d
remove intermediate file dedup
jku20 Jun 25, 2024
bd596e0
remove commented out prints
jku20 Jun 25, 2024
acdbf4e
refactors for readability
jku20 Jul 1, 2024
574e2d6
document IO
jku20 Jul 1, 2024
e711f81
fix typo
jku20 Jul 2, 2024
47699b8
clarify iteration using any
jku20 Jul 2, 2024
29820aa
fix documentation
jku20 Jul 2, 2024
20e3fb4
refactor and document gen_name
jku20 Jul 2, 2024
8d557f5
restore relative generated file path
jku20 Jul 2, 2024
afe6194
refactor gen_name and IO
jku20 Jul 2, 2024
d11dd29
factor out path find code and use enumerate search
jku20 Jul 2, 2024
fbfba5e
convert default impl to macro
jku20 Jul 3, 2024
3b5afb7
add option to choose between old and new plan algo
jku20 Jul 3, 2024
4a7fb63
simplify and document enumeration find_path algo
jku20 Jul 3, 2024
cf3526f
documentation improvements
jku20 Jul 3, 2024
386948d
document FindPath
jku20 Jul 3, 2024
3c28fcf
remove new find_path algorithm
jku20 Jul 3, 2024
c71a6b7
rename path to plan and fix typos
jku20 Jul 8, 2024
26b59ae
more typos
jku20 Jul 8, 2024
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
4 changes: 2 additions & 2 deletions fud2/fud-core/src/exec/data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ entity_impl!(StateRef, "state");
/// An Operation transforms files from one State to another.
pub struct Operation {
pub name: String,
pub input: StateRef,
pub output: StateRef,
pub input: Vec<StateRef>,
pub output: Vec<StateRef>,
pub setups: Vec<SetupRef>,
pub emit: Box<dyn run::EmitBuild>,
}
Expand Down
182 changes: 95 additions & 87 deletions fud2/fud-core/src/exec/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,6 @@ use cranelift_entity::{PrimaryMap, SecondaryMap};
use rand::distributions::{Alphanumeric, DistString};
use std::{collections::HashMap, error::Error, ffi::OsStr, fmt::Display};

#[derive(PartialEq)]
enum Destination {
State(StateRef),
Op(OpRef),
}

type FileData = HashMap<&'static str, &'static [u8]>;

/// A Driver encapsulates a set of States and the Operations that can transform between them. It
Expand All @@ -25,93 +19,107 @@ pub struct Driver {
}

impl Driver {
/// Find a chain of Operations from the `start` state to the `end`, which may be a state or the
/// final operation in the chain.
fn find_path_segment(
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a random drive-by thought for the future: when we do the "bake-off" thing (separating the plan search into a separate module with a well-defined interface, possibly providing multiple implementations), we could include this super simple DFS approach as one possible implementation. It would need to be restricted to situations where every op has exactly 1 input and 1 output, but it could be informative for understanding the cost of multi-input/-output flexibility.

/// Return parents ops of a given state
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment (and possibly name) could be a little clearer: find the set of ops that have this state as an output.

fn parent_ops(&self, state: StateRef) -> Vec<OpRef> {
self.ops
.iter()
.filter_map(|(op_ref, op)| {
if op.output.contains(&state) {
Some(op_ref)
} else {
None
}
})
.collect()
}

fn find_reversed_path(
&self,
start: StateRef,
end: Destination,
) -> Option<Vec<OpRef>> {
// Our start state is the input.
let mut visited = SecondaryMap::<StateRef, bool>::new();
visited[start] = true;

// Build the incoming edges for each vertex.
let mut breadcrumbs = SecondaryMap::<StateRef, Option<OpRef>>::new();

// Breadth-first search.
let mut state_queue: Vec<StateRef> = vec![start];
while !state_queue.is_empty() {
let cur_state = state_queue.remove(0);

// Finish when we reach the goal vertex.
if end == Destination::State(cur_state) {
break;
start: &[StateRef],
end: &[StateRef],
through: &[OpRef],
visited: &mut SecondaryMap<StateRef, bool>,
) -> Option<Vec<(OpRef, Vec<StateRef>)>> {
let mut path: Vec<(OpRef, Vec<StateRef>)> = vec![];
for &output in end.iter().filter(|s| !start.contains(s)) {
// visit the state because it can only be decided upon once
visited[output] = true;

// select and order ops
let ops = self.parent_ops(output);
let mut reordered_ops = vec![];
for &op in through {
if ops.contains(&op) {
reordered_ops.push(op);
}
}

// Traverse any edge from the current state to an unvisited state.
for (op_ref, op) in self.ops.iter() {
if op.input == cur_state && !visited[op.output] {
state_queue.push(op.output);
visited[op.output] = true;
breadcrumbs[op.output] = Some(op_ref);
for op in ops {
if !reordered_ops.contains(&op) {
reordered_ops.push(op);
}
}

// Finish when we reach the goal edge.
if end == Destination::Op(op_ref) {
// recurse to find path to current point
let mut segment = None;
for op in reordered_ops {
if let Some(p) = self.find_reversed_path(
start,
&self.ops[op].input,
through,
visited,
) {
segment = Some((p, op));
break;
}
}
}

// Traverse the breadcrumbs backward to build up the path back from output to input.
let mut op_path: Vec<OpRef> = vec![];
let mut cur_state = match end {
Destination::State(state) => state,
Destination::Op(op) => {
op_path.push(op);
self.ops[op].input
}
};
while cur_state != start {
match breadcrumbs[cur_state] {
Some(op) => {
op_path.push(op);
cur_state = self.ops[op].input;
match segment {
Some((s, o)) => {
let mut in_path = false;
for (op, states) in path.iter_mut() {
if *op == o {
states.push(output);
in_path = true;
break;
}
}
if !in_path {
path.push((o, vec![output]));
}
path.extend(s);
}
None => return None,
}
}
op_path.reverse();

Some(op_path)
Some(path)
}

/// Find a chain of operations from the `start` state to the `end` state, passing through each
/// `through` operation in order.
/// `through` operation in order. Include with each operation the list of used output states.
pub fn find_path(
&self,
start: StateRef,
end: StateRef,
start: &[StateRef],
end: &[StateRef],
through: &[OpRef],
) -> Option<Vec<OpRef>> {
let mut cur_state = start;
let mut op_path: Vec<OpRef> = vec![];

// Build path segments through each through required operation.
for op in through {
let segment =
self.find_path_segment(cur_state, Destination::Op(*op))?;
op_path.extend(segment);
cur_state = self.ops[*op].output;
}

// Build the final path segment to the destination state.
let segment =
self.find_path_segment(cur_state, Destination::State(end))?;
op_path.extend(segment);

Some(op_path)
) -> Option<Vec<(OpRef, Vec<StateRef>)>> {
let path = self.find_reversed_path(
start,
end,
through,
&mut SecondaryMap::new(),
);
// reverse the path
path.map(|mut p| {
p.reverse();
p
})
// make sure we actually go through everything we are trying to go through
// the algo just does it's best to satisfy the through constraint, but doesn't guarrentee it
.filter(|p| {
through
.iter()
.all(|t| p.iter().map(|(o, _)| o).any(|o| o == t))
})
}

/// Generate a filename with an extension appropriate for the given State.
Expand All @@ -132,7 +140,7 @@ impl Driver {
pub fn plan(&self, req: Request) -> Option<Plan> {
// Find a path through the states.
let path =
self.find_path(req.start_state, req.end_state, &req.through)?;
self.find_path(&[req.start_state], &[req.end_state], &req.through)?;

let mut steps: Vec<(OpRef, Utf8PathBuf)> = vec![];

Expand All @@ -144,8 +152,8 @@ impl Driver {
let stem = start_file.file_stem().unwrap();

// Generate filenames for each step.
steps.extend(path.into_iter().map(|op| {
let filename = self.gen_name(stem, self.ops[op].output);
steps.extend(path.into_iter().map(|(op, used_states)| {
let filename = self.gen_name(stem, used_states[0]);
(op, filename)
}));

Expand Down Expand Up @@ -232,8 +240,8 @@ impl Driver {
println!(
" {}: {} -> {}",
op.name,
self.states[op.input].name,
self.states[op.output].name
self.states[op.input[0]].name,
self.states[op.output[0]].name
);
}
}
Expand Down Expand Up @@ -327,15 +335,15 @@ impl DriverBuilder {
&mut self,
name: &str,
setups: &[SetupRef],
input: StateRef,
output: StateRef,
input: &[StateRef],
output: &[StateRef],
emit: T,
) -> OpRef {
self.ops.push(Operation {
name: name.into(),
setups: setups.into(),
input,
output,
input: input.into(),
output: output.into(),
emit: Box::new(emit),
})
}
Expand All @@ -348,7 +356,7 @@ impl DriverBuilder {
output: StateRef,
build: run::EmitBuildFn,
) -> OpRef {
self.add_op(name, setups, input, output, build)
self.add_op(name, setups, &[input], &[output], build)
}

pub fn rule(
Expand All @@ -361,8 +369,8 @@ impl DriverBuilder {
self.add_op(
rule_name,
setups,
input,
output,
&[input],
&[output],
run::EmitRuleBuild {
rule_name: rule_name.to_string(),
},
Expand Down
9 changes: 6 additions & 3 deletions fud2/fud-core/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,11 +151,11 @@ impl<'a> Run<'a> {
let mut ops: HashSet<OpRef> = HashSet::new();
let first_op = self.plan.steps[0].0;
states.insert(
self.driver.ops[first_op].input,
self.driver.ops[first_op].input[0],
self.plan.start.to_string(),
);
for (op, file) in &self.plan.steps {
states.insert(self.driver.ops[*op].output, file.to_string());
states.insert(self.driver.ops[*op].output[0], file.to_string());
ops.insert(*op);
}

Expand All @@ -175,7 +175,10 @@ impl<'a> Run<'a> {

// Show all operations.
for (op_ref, op) in self.driver.ops.iter() {
print!(" {} -> {} [label=\"{}\"", op.input, op.output, op.name);
print!(
" {} -> {} [label=\"{}\"",
op.input[0], op.output[0], op.name
);
Comment on lines +207 to +210
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for this PR, but just making a note that we'll want to come back someday to draw the system as a hypergraph (e.g., ops are square vertices and states are oval vertices or something).

if ops.contains(&op_ref) {
print!(" penwidth=3");
}
Expand Down
8 changes: 7 additions & 1 deletion fud2/fud-core/src/script/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,13 @@ impl ScriptRunner {
ast: Rc::new(sctx.ast.clone_functions_only()),
name: build.fn_name().to_string(),
};
Ok(bld.borrow_mut().add_op(name, &setups, input, output, rctx))
Ok(bld.borrow_mut().add_op(
name,
&setups,
&[input],
&[output],
rctx,
))
},
);
}
Expand Down
86 changes: 86 additions & 0 deletions fud2/fud-core/tests/tests.rs
jku20 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this empty file should be deleted?

Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
use fud_core::DriverBuilder;

#[test]
fn find_path_simple_graph_test() {
let mut bld = DriverBuilder::new("fud2");
let s1 = bld.state("s1", &[]);
let s2 = bld.state("s2", &[]);
let t1 = bld.op("t1", &[], s1, s2, |_, _, _| Ok(()));
let driver = bld.build();
assert_eq!(
Some(vec![(t1, vec![s2])]),
driver.find_path(&[s1], &[s2], &[])
);
assert_eq!(Some(vec![]), driver.find_path(&[s1], &[s1], &[]));
}

#[test]
fn find_path_multi_op_graph() {
let mut bld = DriverBuilder::new("fud2");
let s1 = bld.state("s1", &[]);
let s2 = bld.state("s2", &[]);
let s3 = bld.state("s3", &[]);
let t1 = bld.op("t1", &[], s1, s3, |_, _, _| Ok(()));
let _ = bld.op("t2", &[], s2, s3, |_, _, _| Ok(()));
let driver = bld.build();
assert_eq!(
Some(vec![(t1, vec![s3])]),
driver.find_path(&[s1], &[s3], &[])
);
}

#[test]
fn find_path_multi_path_graph() {
let mut bld = DriverBuilder::new("fud2");
let s1 = bld.state("s1", &[]);
let s2 = bld.state("s2", &[]);
let s3 = bld.state("s3", &[]);
let s4 = bld.state("s4", &[]);
let s5 = bld.state("s5", &[]);
let s6 = bld.state("s6", &[]);
let s7 = bld.state("s7", &[]);
let t1 = bld.op("t1", &[], s1, s3, |_, _, _| Ok(()));
let t2 = bld.op("t2", &[], s2, s3, |_, _, _| Ok(()));
let _ = bld.op("t3", &[], s3, s4, |_, _, _| Ok(()));
let t4 = bld.op("t4", &[], s3, s5, |_, _, _| Ok(()));
let t5 = bld.op("t5", &[], s3, s5, |_, _, _| Ok(()));
let _ = bld.op("t6", &[], s6, s7, |_, _, _| Ok(()));
let driver = bld.build();
assert_eq!(
Some(vec![(t1, vec![s3]), (t4, vec![s5])]),
driver.find_path(&[s1], &[s5], &[])
);
assert_eq!(
Some(vec![(t1, vec![s3]), (t5, vec![s5])]),
driver.find_path(&[s1], &[s5], &[t5])
);
assert_eq!(None, driver.find_path(&[s6], &[s5], &[]));
assert_eq!(None, driver.find_path(&[s1], &[s5], &[t2]));
}

#[test]
fn find_path_only_state_graph() {
let mut bld = DriverBuilder::new("fud2");
let s1 = bld.state("s1", &[]);
let driver = bld.build();
assert_eq!(Some(vec![]), driver.find_path(&[s1], &[s1], &[]));
}

#[test]
fn find_path_cycle_graph() {
let mut bld = DriverBuilder::new("fud2");
let s1 = bld.state("s1", &[]);
let s2 = bld.state("s2", &[]);
let t1 = bld.op("t1", &[], s1, s2, |_, _, _| Ok(()));
let t2 = bld.op("t2", &[], s2, s1, |_, _, _| Ok(()));
let driver = bld.build();
assert_eq!(Some(vec![]), driver.find_path(&[s1], &[s1], &[]));
assert_eq!(
Some(vec![(t1, vec![s2])]),
driver.find_path(&[s1], &[s2], &[])
);
assert_eq!(
Some(vec![(t2, vec![s1])]),
driver.find_path(&[s2], &[s1], &[])
);
}
Loading