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

Fixing minor bug in disjoint_2q_block Layering and adding option add_barriers_between_layers #405

Merged
merged 6 commits into from
Dec 23, 2023

Conversation

EliasLF
Copy link
Collaborator

@EliasLF EliasLF commented Dec 23, 2023

Description

This PR fixes a minor bug in disjoint_2q_block Layering, where it previously pushed 2q-gates erroneously to the second layer, if the first layer has 1q-gates already applied to the same qubits.

I also took this as an opportunity to separate disjoint_2q_block and disjoint_qubits into 2 different functions for the sake of readability. It was already getting cluttered now, and with a soon-to-come suggestion for another similar layering technique (specifically designed for noise-aware mapping) the problem will probably only get worse.
In my opinion, the ensuing code duplication is not too bad, but if you disagree, I am also happy to reintegrate both functions back into one.

Additionally, I added the mapping option add_barriers_between_layers intended for making layer boundaries visible after the mapping process.

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

Copy link

codecov bot commented Dec 23, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b20ddfa) 93.0% compared to head (f358ec1) 93.0%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main    #405   +/-   ##
=====================================
  Coverage   93.0%   93.0%           
=====================================
  Files         46      46           
  Lines       4875    4895   +20     
  Branches     843     849    +6     
=====================================
+ Hits        4534    4554   +20     
  Misses       341     341           
Flag Coverage Δ
cpp 92.7% <100.0%> (+<0.1%) ⬆️
python 96.0% <100.0%> (+<0.1%) ⬆️
Files Coverage Δ
include/Mapper.hpp 83.8% <ø> (ø)
include/configuration/Configuration.hpp 100.0% <ø> (ø)
src/Mapper.cpp 90.3% <100.0%> (+0.4%) ⬆️
src/heuristic/HeuristicMapper.cpp 93.9% <100.0%> (+<0.1%) ⬆️
src/mqt/qmap/compile.py 97.1% <100.0%> (+<0.1%) ⬆️

@EliasLF EliasLF added feature New feature or request c++ Anything related to C++ code fix Anything related to bugfixes labels Dec 23, 2023
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Many thanks. Looking good ✅
Stumbled over two minor things that should be easily addressed or disregarded.

src/Mapper.cpp Outdated Show resolved Hide resolved
test/test_heuristic.cpp Outdated Show resolved Hide resolved
@EliasLF EliasLF merged commit 37232e7 into cda-tum:main Dec 23, 2023
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Anything related to C++ code feature New feature or request fix Anything related to bugfixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants