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

Add slave triggers like we add digitizer triggers. #23

Merged
merged 11 commits into from
May 26, 2016
Merged

Conversation

blakejohnson
Copy link
Collaborator

This introduces a new problem, because sequences aren't decorated with WAITs
until after this is called. So, we need to change the order of the compile
passes for this to work.

@blakejohnson
Copy link
Collaborator Author

While I figure out how to finish this, @ahelsing can use this branch to get rid of the bonus sequence in QGL2 programs.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.5%) to 75.522% when pulling 56a2122 on fix/multiple-trigs into 6bde31a on master.

@blakejohnson
Copy link
Collaborator Author

Now that #25 has landed, I'll generate new test vectors and add them to the PR.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.2%) to 75.614% when pulling 394b74f on fix/multiple-trigs into 1195bc7 on master.

@blakejohnson
Copy link
Collaborator Author

There were a number of more subtle issues here lurking under the covers. The ones I have discovered so far have been fixed. This should pass unit test on Python 2.7 now, but since this involved updating basically every test vector, I am contemplating making a tool to visualize the difference in test vectors prior to merging.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.2%) to 75.614% when pulling 41c1c0a on fix/multiple-trigs into 1195bc7 on master.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.2%) to 75.614% when pulling 26ba818 on fix/multiple-trigs into 1195bc7 on master.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.2%) to 75.169% when pulling c325a57 on fix/multiple-trigs into d58a1c5 on master.

blakejohnson and others added 9 commits May 25, 2016 14:04
This introduces a new problem, because sequences aren't decorated with WAITs
until after this is called. So, we need to change the order of the compile
passes for this to work.
So that slaveTrigger injection has something to latch onto.
Makes a slaveTrig or digitizerTrig less likely to "extend" another pulse when
tensored on.
Replace this expression with an explicit branch.
@blakejohnson blakejohnson changed the title WIP: Add slave triggers like we add digitizer triggers. Add slave triggers like we add digitizer triggers. May 25, 2016
Note: does not yet include the `plot_pulse_files_compare` method because that
needs some cleanup.
@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.6%) to 75.407% when pulling 4a09618 on fix/multiple-trigs into 992c54e on master.

@coveralls
Copy link

coveralls commented May 25, 2016

Coverage Status

Coverage increased (+0.6%) to 75.407% when pulling 7b0005e on fix/multiple-trigs into 992c54e on master.

ct = 0
while ct < len(seq)-1:
if isinstance(seq[ct], ControlFlow.Wait):
if not isinstance(seq[ct+1], ControlFlow.ControlInstruction):
Copy link
Contributor

Choose a reason for hiding this comment

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

@blakejohnson isn't it safer to check if it is a Waveform? E.g. what if it is a BlockLabel?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right that a BlockLabel would be problematic. But, a CompositeWaveform or a PulseBlock would work. Maybe the better approach is just to use a try block and fall back to inserting if the object doesn't support *=.

@coveralls
Copy link

coveralls commented May 26, 2016

Coverage Status

Coverage increased (+0.7%) to 75.518% when pulling f7117ee on fix/multiple-trigs into 992c54e on master.

@blakejohnson
Copy link
Collaborator Author

@caryan is that better?

@caryan
Copy link
Contributor

caryan commented May 26, 2016

With unit tests too!

@caryan caryan merged commit 64a6425 into master May 26, 2016
@caryan caryan deleted the fix/multiple-trigs branch May 26, 2016 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants