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

routing_method='none' and Error pass #5274

Merged
merged 20 commits into from
Dec 9, 2020
Merged

routing_method='none' and Error pass #5274

merged 20 commits into from
Dec 9, 2020

Conversation

1ucian0
Copy link
Member

@1ucian0 1ucian0 commented Oct 21, 2020

Fixes #4053

A new option routing_method='none' in the function qiskit.compiler.transpile.transpile was added to skip
any routing method. If the circuit does not fit coupling map, a qiskit.transpiler.exceptions.TranspilerError
exception will be raised.

The exception is raised via a new Error pass, as suggest in #3604 (comment)

This fixes #4756 as the problem in the issue was caused by the user unexpecting the routing map result. In that situation, the transpilation should be:

qc_joined = qiskit.transpile(qv_circs_joined, backend=backend, routing_method='none', optimization_level=0)

A new option `routing_method='none'` in the function `qiskit.compiler.transpile.transpile` was added to skip
any routing method. If the circuit does not fit coupling map, a `qiskit.transpiler.exceptions.TranspilerError`
exception will be raised.

The expception is raised via a new Error pass
Copy link
Member

@kdk kdk left a comment

Choose a reason for hiding this comment

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

Looks good. Can you add tests for routing_method='none'?

qiskit/transpiler/passes/utils/error.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/utils/error.py Outdated Show resolved Hide resolved
qiskit/transpiler/passes/utils/error.py Outdated Show resolved Hide resolved
qiskit/transpiler/preset_passmanagers/level0.py Outdated Show resolved Hide resolved
@1ucian0
Copy link
Member Author

1ucian0 commented Oct 26, 2020

Looks good. Can you add tests for routing_method='none'?

added in 0be9e9f

Copy link
Member

@ajavadia ajavadia left a comment

Choose a reason for hiding this comment

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

what's the problem exactly that routing_method='none' is trying to address? I had a look at the original issue (#4756) but couldn't really discern the issue. The circuit is not compatible with the backend coupling_map but the user wants to skip routing?

else:
raise TranspilerError('Unknown action: %s' % action)

def run(self, _):
Copy link
Member

Choose a reason for hiding this comment

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

why is this a pass? it doesn't even operate on a circuit. An "ErrorPass" sounds weird. Trapping an error and returning it should happen outside the pass manager IMO.

Copy link
Member Author

Choose a reason for hiding this comment

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

we need a way to raise (in a broad sense) an error if dag is bad for some reason. We already have a way to see if a dag is bad, we have AnalysisPasses. Also, we want passes to "fix" those errors. A pattern like this:

pm.append(CheckProperty())
pm.append(TryToFix(), conditional=lambda ps: ps['is_bad'])
pm.append(CheckProperty())
pm.append(Error(), conditional=lambda ps: ps['is_bad'])

That pattern is similar to what happens here, when routing_method='none':

pm.append(CheckMap(coupling_map))
pm.append(Error(), condition=lambda property_set: not property_set['is_swap_mapped'])

Alternative, we could have ValidationPasss like suggested in #3604 . However, that will duplicate code with the AnalysisPass sometimes. For example, a ValidationMap pass (raises when the dag is not mapped) would have practically the same code as CheckMap.

To avoid the duplication, ValidationMap could require CheckMap. In that case, ValidationMap will also not operate on the circuit, but the property set. More relevantly, this approach will push the condition to check into the pass (instead of the passmanager condition). I think that's a fine solution, but it might break in a lot of validation passes with time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Other possible examples for validation pass:

@kdk kdk linked an issue Oct 27, 2020 that may be closed by this pull request
@1ucian0
Copy link
Member Author

1ucian0 commented Oct 28, 2020

what's the problem exactly that routing_method='none' is trying to address? I had a look at the original issue (#4756) but couldn't really discern the issue. The circuit is not compatible with the backend coupling_map but the user wants to skip routing?

@kdk linked #4053 about a similar discussion. I think there is value for the user to make sure no routing method is running, specially if the user runs a batch of circuits (likes in #4756) where a "bad" (as non compatible with the backend) circuit slips in.

@kdk kdk added automerge Changelog: New Feature Include in the "Added" section of the changelog labels Dec 8, 2020
@kdk kdk added this to the 0.17 milestone Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transpiler is reordering classical bit locations Add transpile option to disable routing
3 participants