Fix: Instruction used/blocked frames (#73)

* Add private method for parsing a single instruction from a string

* Fix: instruction frame blocking

* Fix: frame dependency calculation in ScheduledProgram

* Fix how FENCE uses frames

* Add Instruction.get_frame_match_condition

Author: kalzoo

src/instruction.rs

@@ -16,6 +16,7 @@ use serde::{Deserialize, Serialize};
 use std::{collections::HashMap, fmt};
 
 use crate::expression::Expression;
+use crate::program::frame::FrameMatchCondition;
 
 #[cfg(test)]
 use proptest_derive::Arbitrary;
@@ -941,6 +942,90 @@ impl Instruction {
             _ => {}
         }
     }
+
+    pub(crate) fn get_frame_match_condition(
+        &self,
+        include_blocked: bool,
+    ) -> Option<FrameMatchCondition> {
+        match self {
+            Instruction::Pulse(Pulse {
+                blocking, frame, ..
+            })
+            | Instruction::Capture(Capture {
+                blocking, frame, ..
+            })
+            | Instruction::RawCapture(RawCapture {
+                blocking, frame, ..
+            }) => Some(if *blocking && include_blocked {
+                FrameMatchCondition::AnyOfQubits(&frame.qubits)
+            } else {
+                FrameMatchCondition::Specific(frame)
+            }),
+            Instruction::Delay(Delay {
+                frame_names,
+                qubits,
+                ..
+            }) => Some(if frame_names.is_empty() {
+                FrameMatchCondition::ExactQubits(qubits)
+            } else {
+                FrameMatchCondition::And(vec![
+                    FrameMatchCondition::ExactQubits(qubits),
+                    FrameMatchCondition::AnyOfNames(frame_names),
+                ])
+            }),
+            Instruction::Fence(Fence { qubits }) => Some(if qubits.is_empty() {
+                FrameMatchCondition::All
+            } else {
+                FrameMatchCondition::AnyOfQubits(qubits)
+            }),
+            Instruction::SetFrequency(SetFrequency { frame, .. })
+            | Instruction::SetPhase(SetPhase { frame, .. })
+            | Instruction::SetScale(SetScale { frame, .. })
+            | Instruction::ShiftFrequency(ShiftFrequency { frame, .. })
+            | Instruction::ShiftPhase(ShiftPhase { frame, .. }) => {
+                Some(FrameMatchCondition::Specific(frame))
+            }
+            Instruction::SwapPhases(SwapPhases { frame_1, frame_2 }) => {
+                Some(FrameMatchCondition::And(vec![
+                    FrameMatchCondition::Specific(frame_1),
+                    FrameMatchCondition::Specific(frame_2),
+                ]))
+            }
+            Instruction::Gate(_)
+            | Instruction::CircuitDefinition(_)
+            | Instruction::GateDefinition(_)
+            | Instruction::Declaration(_)
+            | Instruction::Measurement(_)
+            | Instruction::Reset(_)
+            | Instruction::CalibrationDefinition(_)
+            | Instruction::FrameDefinition(_)
+            | Instruction::MeasureCalibrationDefinition(_)
+            | Instruction::Pragma(_)
+            | Instruction::WaveformDefinition(_)
+            | Instruction::Arithmetic(_)
+            | Instruction::Halt
+            | Instruction::Label(_)
+            | Instruction::Move(_)
+            | Instruction::Exchange(_)
+            | Instruction::Load(_)
+            | Instruction::Store(_)
+            | Instruction::Jump(_)
+            | Instruction::JumpWhen(_)
+            | Instruction::JumpUnless(_) => None,
+        }
+    }
+
+    #[cfg(test)]
+    /// Parse a single instruction from an input string. Returns an error if the input fails to parse,
+    /// or if there is input left over after parsing.
+    pub(crate) fn parse(input: &str) -> Result<Self, String> {
+        use crate::parser::{instruction::parse_instruction, lex};
+
+        let lexed = lex(input)?;
+        let (_, instruction) =
+            nom::combinator::all_consuming(parse_instruction)(&lexed).map_err(|e| e.to_string())?;
+        Ok(instruction)
+    }
 }
 
 #[cfg(test)]

src/parser/mod.rs

@@ -27,7 +27,7 @@ mod macros;
 mod common;
 mod error;
 mod expression;
-mod instruction;
+pub(crate) mod instruction;
 mod lexer;
 
 type ParserInput<'a> = &'a [Token];

src/program/frame.rs

@@ -34,18 +34,47 @@ impl FrameSet {
         self.frames.keys().collect()
     }
 
-    /// Return all contained FrameIdentifiers which include **exactly** these qubits (in any order)
-    /// and - if names are provided - match one of the names.
-    pub fn get_matching_keys(&self, qubits: &[Qubit], names: &[String]) -> Vec<&FrameIdentifier> {
-        let qubit_set: HashSet<&Qubit> = qubits.iter().collect();
-        self.frames
-            .iter()
-            .filter(|(identifier, _)| {
-                (names.is_empty() || names.contains(&identifier.name))
-                    && qubit_set == identifier.qubits.iter().collect()
-            })
-            .map(|(id, _)| id)
-            .collect::<Vec<_>>()
+    /// Return all frames in the set which match all of these conditions. If a frame _would_ match, but is
+    /// not present in this [FrameSet], then it is not returned (notably, the [FrameMatchCondition::Specific]
+    /// match condition).
+    pub(crate) fn get_matching_keys<'s, 'a>(
+        &'s self,
+        condition: FrameMatchCondition<'a>,
+    ) -> HashSet<&'s FrameIdentifier> {
+        let keys = self.frames.keys();
+
+        match condition {
+            FrameMatchCondition::All => keys.collect(),
+            FrameMatchCondition::AnyOfNames(names) => {
+                keys.filter(|&f| names.contains(&f.name)).collect()
+            }
+            FrameMatchCondition::AnyOfQubits(qubits) => {
+                let any_of_set: HashSet<_> = qubits.iter().collect();
+
+                keys.filter(|&f| f.qubits.iter().any(|q| any_of_set.contains(q)))
+                    .collect()
+            }
+            FrameMatchCondition::ExactQubits(qubits) => {
+                let exact_set: HashSet<_> = qubits.iter().collect();
+
+                keys.filter(|&f| f.qubits.iter().collect::<HashSet<_>>() == exact_set)
+                    .collect()
+            }
+            FrameMatchCondition::Specific(frame) => {
+                // This unusual pattern (fetch key & value by key, discard value) allows us to return
+                // a reference to `self` rather than `condition`, keeping lifetimes simpler.
+                if let Some((frame, _)) = self.frames.get_key_value(frame) {
+                    vec![frame].into_iter().collect()
+                } else {
+                    HashSet::new()
+                }
+            }
+            FrameMatchCondition::And(conditions) => conditions
+                .into_iter()
+                .map(|c| self.get_matching_keys(c))
+                .reduce(|acc, el| acc.into_iter().filter(|&v| el.contains(v)).collect())
+                .unwrap_or_default(),
+        }
     }
 
     /// Retrieve the attributes of a frame by its identifier.
@@ -86,3 +115,23 @@ impl FrameSet {
             .collect()
     }
 }
+
+pub(crate) enum FrameMatchCondition<'a> {
+    /// Match all frames in the set
+    All,
+
+    /// Match all frames which share any one of these names
+    AnyOfNames(&'a [String]),
+
+    /// Match all frames which contain any of these qubits
+    AnyOfQubits(&'a [Qubit]),
+
+    /// Match all frames which contain exactly these qubits
+    ExactQubits(&'a [Qubit]),
+
+    /// Return these specific frames, if present in the set
+    Specific(&'a FrameIdentifier),
+
+    /// Return all frames which match all of these conditions
+    And(Vec<FrameMatchCondition<'a>>),
+}

src/program/graph.rs

@@ -233,19 +233,26 @@ impl InstructionBlock {
                     Ok(())
                 }
                 InstructionRole::RFControl => {
-                    let frames = match program.get_frames_for_instruction(instruction, true) {
-                        Some(frames) => frames,
-                        None => vec![],
-                    };
-
-                    // Mark a dependency on the last instruction which executed in the context of each target frame
-                    for frame in frames {
+                    let used_frames = program
+                        .get_frames_for_instruction(instruction, false)
+                        .unwrap_or_default();
+                    let blocked_frames = program
+                        .get_frames_for_instruction(instruction, true)
+                        .unwrap_or_default();
+
+                    // Take a dependency on any previous instructions to _block_ a frame which this instruction _uses_.
+                    for frame in used_frames {
                         let previous_node_id = last_instruction_by_frame
-                            .entry(frame.clone())
-                            .or_insert(ScheduledGraphNode::BlockStart);
+                            .get(frame)
+                            .unwrap_or(&ScheduledGraphNode::BlockStart);
                         add_dependency!(graph, *previous_node_id => node, ExecutionDependency::ReferenceFrame);
+                    }
+
+                    // We mark all "blocked" frames as such for later instructions to take a dependency on
+                    for frame in blocked_frames {
                         last_instruction_by_frame.insert(frame.clone(), node);
                     }
+
                     Ok(())
                 }
                 InstructionRole::ControlFlow => Err(ScheduleError {

src/program/graphviz_dot.rs

@@ -233,6 +233,8 @@ DEFFRAME 0 \"ro_rx\":
     INITIAL-FREQUENCY: 1e6
 DEFFRAME 0 \"ro_tx\":
     INITIAL-FREQUENCY: 1e6
+DEFFRAME 0 1 \"cz\":
+    INITIAL-FREQUENCY: 1e6
 ";
 
                     let program =
@@ -308,8 +310,19 @@ NONBLOCKING PULSE 0 \"rf\" test(duration: 1e6)
 NONBLOCKING PULSE 1 \"rf\" test(duration: 1e6)
 "
         );
+
         build_dot_format_snapshot_test_case!(fence_all, "FENCE");
 
+        build_dot_format_snapshot_test_case!(
+            fence_wrapper,
+            "
+FENCE
+NONBLOCKING PULSE 0 1 \"cz\" test(duration: 1e-6)
+NONBLOCKING PULSE 1 \"rf\" test(duration: 1e-6)
+FENCE 1
+"
+        );
+
         build_dot_format_snapshot_test_case!(
             jump,
             "DECLARE ro BIT