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

Stop shuffling coupling map node indices in VF2 passes #13492

Merged
merged 5 commits into from
Jan 10, 2025
Merged

Stop shuffling coupling map node indices in VF2 passes #13492

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bdaff35
Stop shuffling coupling map node indices in VF2 passes
mtreinish Nov 25, 2024
aacc29e
Fix expected value in primitives test
mtreinish Jan 6, 2025
293dafa
Merge remote-tracking branch 'origin/main' into vf2-in-order
mtreinish Jan 6, 2025
0c8ccb8
Restore removed argument functionality and change default instead
mtreinish Jan 6, 2025
f3cfca7
Fix lint
mtreinish Jan 7, 2025
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
6 changes: 3 additions & 3 deletions qiskit/transpiler/preset_passmanagers/builtin_plugins.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ def _swap_mapped(property_set):
)
choose_layout_1 = VF2Layout(
coupling_map=pass_manager_config.coupling_map,
seed=pass_manager_config.seed_transpiler,
seed=-1,
call_limit=int(5e4), # Set call limit to ~100ms with rustworkx 0.10.2
properties=pass_manager_config.backend_properties,
target=pass_manager_config.target,
Expand Down Expand Up @@ -826,7 +826,7 @@ def _swap_mapped(property_set):
elif optimization_level == 2:
choose_layout_0 = VF2Layout(
coupling_map=pass_manager_config.coupling_map,
seed=pass_manager_config.seed_transpiler,
seed=-1,
call_limit=int(5e6), # Set call limit to ~10s with rustworkx 0.10.2
properties=pass_manager_config.backend_properties,
target=pass_manager_config.target,
Expand Down Expand Up @@ -861,7 +861,7 @@ def _swap_mapped(property_set):
elif optimization_level == 3:
choose_layout_0 = VF2Layout(
coupling_map=pass_manager_config.coupling_map,
seed=pass_manager_config.seed_transpiler,
seed=-1,
call_limit=int(3e7), # Set call limit to ~60s with rustworkx 0.10.2
properties=pass_manager_config.backend_properties,
target=pass_manager_config.target,
Expand Down
2 changes: 1 addition & 1 deletion qiskit/transpiler/preset_passmanagers/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def _swap_condition(property_set):
target,
coupling_map,
backend_properties,
seed_transpiler,
Copy link
Contributor

Choose a reason for hiding this comment

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

Lint caught that the seed_transpiler argument of generate_routing_passmanager is not being used anymore. We should probably not have it as an input argument if it's not going to be used, but I feel like this might make the change more... permanent? We at least should make sure we want to stick to the fixed seed. On the other hand, I've checked and it looks like generate_routing_passmanager is not part of the public API, so technically we could remove the argument without deprecation, but I see this as another argument to leave this PR for 2.0.

Copy link
Member Author

@mtreinish mtreinish Nov 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It is unfortunately a public function: https://docs.quantum.ibm.com/api/qiskit/transpiler_preset#generate_routing_passmanager so we can't just change it. We could in 2.0 with an appropriate deprecation added in 1.4. Personally for this case if we were to include it in 1.3 I'd just disable the lint with a comment about preserving the signature.

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 opted to restore this argument's functionality and instead change the default to -1 to not use any randomization. Since we're in 2.0 I figured a change in default was the least bad option here. I'm still not convinced there is a use case where the randomization is actually helping because in all the testing I've done it only just makes it harder for vf2 to find matches, but I can understand people wanting to experiment with this and it's why I didn't remove it from the pass itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this approach makes sense!

seed=-1,
call_limit=vf2_call_limit,
max_trials=vf2_max_trials,
strict_direction=False,
Expand Down
Loading