Fix 'from address too close' behavior in dac.py

Improve documentation for jump_table in dac.py
Add test_dac
Change dac build 15 offsets:
*JT_MIN_FROM_ADDR_SPACING -> JT_MIN_CLK_CYCLES_BETWEEN_OPCODES
*JT_MIN_CLK_CYCLES_BETWEEN_OPCODES: 2->4 in line with GHZDAC8 specs
*JT_MIN_FROM_ADDR remove offset
*JT_MIN_END_ADDR remove offset
Fixes #323

Review: @maffoo

fpgalib/dac.py

@@ -1172,6 +1172,23 @@ class DAC_Build15(DAC_Build8):
     19  d[1]
     ...
     33  d[15]
+
+    Table documenting how each op code can move the sram, jump table index
+
+    OP    | BEHAVIOR
+    ------------------------------------------------------------------------
+    IDLE  | increment sram_idx and increment jt_idx
+    NOP   | increment sram_idx and increment jt_idx
+    CHECK | move sram_idx and jt_idx OR increment sram_idx and jt_idx
+    JUMP  | move sram_idx and jt_idx
+    CYCLE | move sram_idx and jt_idx OR increment sram_idx and jt_idx
+    END   | no next entry
+
+    Attributes:
+        increment_ops (list[jump_table.Operation]): Jump table op codes that can
+            increment the sram index, jump table index
+        jump_ops (list[jump_table.Operation]): Jump table op codes that can
+            jump the sram index, jump table index
     """
 
     HAS_JUMP_TABLE = True
@@ -1193,9 +1210,9 @@ class DAC_Build15(DAC_Build8):
     JT_IDLE_OFFSET = -0
     JT_IDLE_MIN = 0 - JT_IDLE_OFFSET
     JT_IDLE_MAX = 2**IDLE_BITS - 1 - JT_IDLE_OFFSET
-    JT_MIN_FROM_ADDR_SPACING = 2
-    JT_MIN_FROM_ADDR = JT_MIN_FROM_ADDR_SPACING - JT_FROM_ADDR_OFFSET
-    JT_MIN_END_ADDR = JT_MIN_FROM_ADDR_SPACING - JT_END_ADDR_OFFSET
+    JT_MIN_CLK_CYCLES_BETWEEN_OPCODES = 4
+    JT_MIN_FROM_ADDR = JT_MIN_CLK_CYCLES_BETWEEN_OPCODES
+    JT_MIN_END_ADDR = JT_MIN_CLK_CYCLES_BETWEEN_OPCODES
     JT_MIN_TO_ADDR = 0
     JT_MAX_FROM_ADDR = SRAM_LEN // 4 + JT_FROM_ADDR_OFFSET
     JT_MAX_END_ADDR = SRAM_LEN // 4 + JT_END_ADDR_OFFSET
@@ -1205,6 +1222,15 @@ class DAC_Build15(DAC_Build8):
     MONITOR_0 = 5
     MONITOR_1 = 10
 
+    increment_ops = (
+        jump_table.IDLE, jump_table.NOP, jump_table.CHECK,
+        jump_table.CYCLE
+    )
+
+    jump_ops = (
+        jump_table.CHECK, jump_table.JUMP, jump_table.CYCLE
+    )
+
     @classmethod
     def convert_from_address(cls, x_ns):
         """ Convert a from address from ns to SRAM index.
@@ -1430,7 +1456,7 @@ def make_jump_table_entry(cls, name, arg):
         :param str name: one of: END, NOP, IDLE, CYCLE, JUMP, CHECK, RAMP
         :param list[int] arg: arguments for the op.
         :return: list of jump table entries
-        :rtype: list[jump_table.JumpEntry]
+        :rtype: jump_table.JumpEntry
         """
         if name == 'CHECK':
             raise NotImplementedError("Check not implemented yet")
@@ -1477,19 +1503,42 @@ def make_jump_table_entry(cls, name, arg):
     def make_jump_table(cls, jt_entries, counters=None, start_address_ns=0):
         """Make a jump table out of the given entries and counters.
 
+        We verify that jump table commands are not executed too frequently. We
+        check when our current command executes, and compare to execution of
+        the next command. The next command depends on the specifics of the
+        current command, arguments and conditions. See attributes increment_ops,
+        jump_ops.
+
         :param list[jump_table.JumpEntry] jt_entries: JT entries
         :param list[int] counters: counter values, or None for all 0s
         :param int start_address_ns: SRAM start address, in ns
         :return: jump table object
         :rtype: jump_table.JumpTable
         """
-        for i, a in enumerate(jt_entries):
-            for j, b in enumerate(jt_entries[i+1:]):
-                if abs(a.from_addr - b.from_addr) < cls.JT_MIN_FROM_ADDR_SPACING:
-                    raise ValueError(
-                        "Entries {}({}) and {}({}) have from addrstoo close together.".format(
-                            i, a.operation, i+j+1, b.operation)
-                    )
+
+        # no two opcodes executed within JT_MIN_CLK_CYCLES_BETWEEN_OPCODES
+        for k, jt_entry in enumerate(jt_entries):
+            # first pass we check all increment cases
+            if isinstance(jt_entry.operation, cls.increment_ops):
+                sram_idx = jt_entry.from_addr
+                next_jt_idx = k + 1
+                next_from_addr = jt_entries[next_jt_idx].from_addr
+                if ((next_from_addr - sram_idx) <
+                        cls.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES):
+                    raise ValueError("Entry {} and entry it increments to "
+                                     "executed too closely in time"
+                                     .format(jt_entry))
+            # second pass we check all jump cases
+            if isinstance(jt_entry.operation, cls.jump_ops):
+                sram_idx = jt_entry.to_addr
+                # take off 1 as first entry is always NOP (self.toString)
+                next_jt_idx = jt_entry.operation.jump_index - 1
+                next_from_addr = jt_entries[next_jt_idx].from_addr
+                if ((next_from_addr - sram_idx) <
+                        cls.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES):
+                    raise ValueError("Entry {} and entry it jumps to executed "
+                                     "too closely in time".format(jt_entry))
+
         return jump_table.JumpTable(
             start_addr=cls.convert_to_address(start_address_ns),
             jumps=jt_entries,

fpgalib/jump_table.py

@@ -27,6 +27,8 @@ def __init__(self, from_addr, to_addr, operation):
         :param int to_addr: the to address (should be 0 if not used)
         :param Operation operation: the operation
         """
+
+        # TODO: Why is this -1 hard coded?
         if abs(from_addr - to_addr) - 1 == 0:
             raise ValueError(
                 "from_addr: {} and to_addr: {} are too close".format(
@@ -114,6 +116,8 @@ class CHECK(Operation):
         which_daisy_bit (int): Which daisy bit to check.
             Need to explain how the numbering works.
         jump_index (int): Index of jump entry to activte after the check.
+             Note: jump_table gets an extra NOP start entry at run time in
+            self.toString, so first user entry is index 1.
         bit_state (bool): Selects whether to fire on 0 or 1.
         Need to explain more.
 
@@ -153,7 +157,8 @@ class JUMP(Operation):
 
     Attributes:
         jump_index (int): Index of jump table entry to activate after
-        this one.
+            this one. Note: jump_table gets an extra NOP start entry at run time
+            in self.toString, so first user entry is index 1.
     """
     NAME = "JUMP"
 
@@ -195,8 +200,9 @@ class CYCLE(Operation):
     """Cycle back to another SRAM address.
 
     Attributes:
-    jump_index (int): Index of jump table entry to activate after firing
-        this one.
+        jump_index (int): Index of jump table entry to activate after firing
+            this one. Note: jump_table gets an extra NOP start entry at run time
+            in self.toString, so first user entry is index 1.
     counter (int): Which counter to increment at each cycle.
     """
     NAME = "CYCLE"

fpgalib/test/test_dac.py

@@ -0,0 +1,103 @@
+"""This is intended to test fpgalib/dac.py"""
+
+import pytest
+import fpgalib.dac as dac
+
+
+class TestDAC15(object):
+    @classmethod
+    def setup_class(cls):
+        cls.dac = dac.DAC_Build15(0, 'dac_name')
+
+        # sequence SRAM: pi pulse from 0-32, pi pulse from 32-64, ro from 64-400
+        # experiment: first pi -> measure -> second pi -> measure -> end
+        cls.first_pi_start = 0
+        cls.first_pi_end = 32
+        cls.second_pi_start = 32
+        cls.second_pi_end = 64
+        cls.ro_start = 64
+        cls.ro_end = 400
+
+        cls.jump_args = [
+            ('JUMP', [cls.first_pi_end, cls.ro_start, 1]),
+            ('JUMP', [cls.ro_end, cls.second_pi_start, 2]),
+            ('END', [cls.ro_end])
+        ]
+
+    def make_entries(self, jump_args):
+        return [self.dac.make_jump_table_entry(name, args)
+                for name, args in jump_args]
+
+    def test_make_jump_table_entry(self):
+        _ = self.make_entries(self.jump_args)
+        # we made it this far and we're feeling pretty good about it
+
+    def test_make_jump_table(self):
+        entries = self.make_entries(self.jump_args)
+        self.dac.make_jump_table(entries)
+        # we made it this far and we're feeling pretty good about it
+
+    def test_catch_nop_nop_too_close(self):
+        entries = self.make_entries([
+            ('NOP', [16]),
+            ('NOP', [20]),
+            ('END', [400])
+        ])
+        with pytest.raises(ValueError):
+            self.dac.make_jump_table(entries)
+
+    def test_catch_jump_iterated_nop_too_close(self):
+
+        entries = self.make_entries([
+            ('JUMP', [32, 16, 1]),
+            ('NOP', [20]),
+            ('END', [400])
+        ])
+        with pytest.raises(ValueError):
+            self.dac.make_jump_table(entries)
+
+    def test_catch_jump_back_too_close(self):
+
+        entries = self.make_entries([
+            ('NOP', [32]),
+            ('JUMP', [100, 28, 0]),
+            ('END', [400])
+        ])
+        with pytest.raises(ValueError):
+            self.dac.make_jump_table(entries)
+
+    def test_catch_idle_end_too_close(self):
+
+        # from addr of 32 is followed by from addr of 36 (even though IDLE?)
+        entries = self.make_entries([
+            ('IDLE', [32, 200]),
+            ('END', [36])
+        ])
+        with pytest.raises(ValueError):
+            self.dac.make_jump_table(entries)
+
+    def test_nop_nop_1_clk_too_close(self):
+        """Test a table that executes commands 1 clock cycle too frequently"""
+
+        from_addr_ns = self.dac.JT_MIN_FROM_ADDR * 4
+        entries = self.make_entries([
+            ('NOP', [from_addr_ns]),
+            ('NOP', [from_addr_ns +
+                     (self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES - 1) * 4]),
+            ('END', [400])
+        ])
+
+        with pytest.raises(ValueError):
+            self.dac.make_jump_table(entries)
+
+    def test_nop_nop_min_clk_ok(self):
+        from_addr_ns = self.dac.JT_MIN_FROM_ADDR * 4
+        _ = self.make_entries([
+            ('NOP', [from_addr_ns]),
+            ('NOP', [from_addr_ns +
+                     self.dac.JT_MIN_CLK_CYCLES_BETWEEN_OPCODES * 4]),
+            ('END', [400])
+        ])
+
+if __name__ == '__main__':
+    pytest.main(['-v', __file__])