Skip to content

changing the default :initarg of channel-qvm to be valid - #211

Merged
stylewarning merged 1 commit into
masterfrom
bugfix/channel-qvm-default-initargs
Nov 8, 2019
Merged

stylewarning merged 1 commit into
masterfrom
bugfix/channel-qvm-default-initargs

Conversation

@sophiaponte

Copy link
Copy Markdown
Contributor

#210

The channel-qvm expects its :noise-model slot to be a noise-model object, but it's default initarg is nil. This causes errors when a channel-qvm is created without a defined noise-model argument. To fix this I:

  • changed the default noise-model initarg of channel-qvm to be a noise-model with an empty set of noise-rules

  • I added a check to initialize-instance to ensure that the noise-model slot of channel-qvmis of typenoise-model`

  • added checks to a test in tests/channel-qvm

…l. Also, adding a check-type to ensure that the noise-model argument of a channel-qvm is a valid noise model. Adding tests for this change.
@sophiaponte
sophiaponte requested a review from a team as a code owner November 8, 2019 20:27
@rigettizach

Copy link
Copy Markdown
Contributor

changed the default noise-model initarg of channel-qvm to be a noise-model with an empty set of noise-rules

Is there any risk this could trip up someone who meant to provide a non-empty noise model but forgot?

@sophiaponte

Copy link
Copy Markdown
Contributor Author

Is there any risk this could trip up someone who meant to provide a non-empty noise model but forgot?

Yes, I suppose if someone forgot to provide a noise model, the channel-qvm would run with the default empty noise model as if there were no noise.

So I guess an alternative option is to make the noise model a required argument to the qvm -- that way, the noise model will only be empty if the user explicitly wants it so.

@appleby appleby left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Normally I would lean towards Zach's suggestion to make noise-model a required argument, but all the other noisy qvm types seem to default to "empty noise" if a caller doesn't provide any noise, so I think it's reasonable for channel-qvm to follow suit here.

@stylewarning
stylewarning merged commit 2a88270 into master Nov 8, 2019
@stylewarning
stylewarning deleted the bugfix/channel-qvm-default-initargs branch November 8, 2019 23:03
@notmgsk notmgsk mentioned this pull request Nov 26, 2019
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.

4 participants