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

Jump to bottom

Linspace bug follow up with no bugs #862

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Linspace bug follow up with no bugs #862

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bb874cc
first failing test for pre_dep!=post_dep
Nomos11 Jun 20, 2024
655f107
Correct type annotation in LinspaceVM
terrorfisch Jun 21, 2024
98d2a01
Cleanup test. Originial commands and program work in simulator
terrorfisch Jun 21, 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
12 changes: 8 additions & 4 deletions qupulse/program/linspace.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,7 @@ def __init__(self, channels: int):
self.time = TimeType(0)
self.registers = tuple({} for _ in range(channels))

self.history: List[Tuple[TimeType, Tuple[float, ...]]] = []
self.history: List[Tuple[TimeType, List[float]]] = []

self.commands = None
self.label_targets = None
Expand All @@ -430,9 +430,13 @@ def change_state(self, cmd: Union[Set, Increment, Wait, Play]):
if isinstance(cmd, Play):
raise NotImplementedError("TODO: Implement arbitrary waveform simulation")
elif isinstance(cmd, Wait):
self.history.append(
(self.time, self.current_values.copy())
)
if self.history and self.history[-1][1] == self.current_values:
# do not create noop entries
pass
else:
self.history.append(
(self.time, self.current_values.copy())
)
self.time += cmd.duration
elif isinstance(cmd, Set):
self.current_values[cmd.channel] = cmd.value
Expand Down
75 changes: 75 additions & 0 deletions tests/program/linspace_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,81 @@ def test_output(self):
assert_vm_output_almost_equal(self, self.output, vm.history)


class PrePostDepTest(TestCase):
def setUp(self):
hold = ConstantPT(10 ** 6, {'a': '-1. + idx * 0.01'})
hold_random = ConstantPT(10 ** 5, {'a': -.4})
# self.pulse_template = (hold_random@(hold_random@hold).with_repetition(10)@hold_random@hold)\
self.pulse_template = (hold_random @ hold.with_repetition(10)).with_iteration('idx', 200)

self.program = LinSpaceIter(
length=200,
body=(
LinSpaceHold(bases=(-.4,), factors=(None,), duration_base=TimeType(10**5), duration_factors=None),
LinSpaceRepeat(body=(
LinSpaceHold(bases=(-1.,),factors=((0.01,),),duration_base=TimeType(10**6),duration_factors=None),
), count=10),
# LinSpaceHold(bases=(-.4),factors=None,duration_base=TimeType(10**6),duration_factors=None),
# LinSpaceHold(bases=(-1.,),factors=((0.01,),),duration_base=TimeType(10**6),duration_factors=None)
),)


key = DepKey.from_voltages((0.01,), DEFAULT_INCREMENT_RESOLUTION)

self.commands = [
Set(channel=0, value=-0.4, key=DepKey(factors=())),
Wait(duration=TimeType(100000, 1)),
#here is what currently happens:
Set(channel=0, value=-1.0, key=DepKey(factors=(10000000,))),
Wait(duration=TimeType(1000000, 1)),
LoopLabel(idx=0, count=9),
Wait(duration=TimeType(1000000, 1)),
LoopJmp(idx=0),
#however, i think this is what should happen (maybe also with an additional "Set" before,
#which might cause complications if omitted in other contexts like AWG amplitude:
#LoopLabel(idx=0, count=10),
#Set(channel=0, value=-1.0, key=DepKey(factors=(10000000,))),
Copy link
Member Author

Choose a reason for hiding this comment

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

This set statement is not required. because it is the same in every iteration and can be lifted out of the loop. Why should it happen? This is exactly one intended case the post != pre condition is supposed to detect and optimize.

Copy link
Collaborator

Choose a reason for hiding this comment

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

on further consideration, I think the issue that I saw when I first thought about this - that for long sequences that get repeated, this whole block is basically written twice in the sequence code, first for initial Setting, then forlooping n-1 times - is less of an issue overall if (what I did some time after that) e.g. for the HDAWG the entries for the CT can be recycled for the second code block.

Copy link
Member Author

Choose a reason for hiding this comment

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

the thing is that it is required to do this twice because the initial state changes. I brooded over this quite a while I remember :D But it might still be that I am wrong...

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don‘t think you‘re wrong; just in the very pathological case of repeating a sequence of 1000 pulses, where one element is just „Set“ with a Set command, one may want to prefer to have this codeblock only once and does not want that perceived optimization of omitting the Set command but having to write the repeated sequence twice.
But I don‘t think this is very relevant.

#Wait(duration=TimeType(1000000, 1)),
#LoopJmp(idx=0),

LoopLabel(idx=1, count=199),
Set(channel=0, value=-0.4, key=DepKey(factors=())),
Wait(duration=TimeType(100000, 1)),
Increment(channel=0, value=0.01, dependency_key=DepKey(factors=(10000000,))),
Wait(duration=TimeType(1000000, 1)),
LoopLabel(idx=2, count=9),
#also here, an increment 0 may be helpful (at least to be able to force).
Copy link
Member Author

Choose a reason for hiding this comment

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

The increment is not emitted due to an optimization. Why would it be helpful?

Copy link
Member Author

Choose a reason for hiding this comment

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

I would go further and replace loops that are constant by waits

Copy link
Collaborator

Choose a reason for hiding this comment

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

for wait it's not helpful. Just for other things, like WF scale, the program would need to know about the register it has to choose, which may work smoother if the corresponding Set command with the relevant DepKey is directly in front of it. Or maybe not, I dont' know.

Copy link
Member Author

@terrorfisch terrorfisch Jun 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think a scaled play should have a dependency key.

#Increment(channel=0, value=0, dependency_key=DepKey(factors=(10000000,))),
Wait(duration=TimeType(1000000, 1)),
LoopJmp(idx=2),
LoopJmp(idx=1)
]

self.output = []
time = TimeType(0)
for idx in range(200):
self.output.append((time, [-.4]))
time += TimeType(10**5)
self.output.append((time, [-1. + idx * 0.01]))
time += TimeType(10**7)


def test_program(self):
program_builder = LinSpaceBuilder(('a',))
program = self.pulse_template.create_program(program_builder=program_builder)
self.assertEqual([self.program], program)

def test_commands(self):
commands = to_increment_commands([self.program])
self.assertEqual(self.commands, commands)

def test_output(self):
vm = LinSpaceVM(1)
vm.set_commands(self.commands)
vm.run()
assert_vm_output_almost_equal(self, self.output, vm.history)


class PlainCSDTest(TestCase):
def setUp(self):
hold = ConstantPT(10**6, {'a': '-1. + idx_a * 0.01', 'b': '-.5 + idx_b * 0.02'})
Expand Down
Loading