Skip to content

♻️ Own neutral-atom OpenQASM serialization - #1124

Draft
simon1hofmann wants to merge 3 commits into
na-transferfrom
codex/2098-openqasm-serializer
Draft

♻️ Own neutral-atom OpenQASM serialization#1124
simon1hofmann wants to merge 3 commits into
na-transferfrom
codex/2098-openqasm-serializer

Conversation

@simon1hofmann

@simon1hofmann simon1hofmann commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

🤖 AI text below 🤖

Description

Adapt the neutral-atom extraction in #1111 to the dedicated serializer from
MQT Core #2249.

  • add QMAP-owned extended OpenQASM 2 serialization for MOVE, BRIDGE, and AOD
    operations;
  • let MQT Core traverse compounds and conditionals, while QMAP handles only its
    custom leaf operations;
  • remove OpenQASM overrides from QMAP operation subclasses;
  • migrate hybrid mapper output and the superconducting DataLogger;
  • preserve the existing extended OpenQASM output with exact-output tests.

This is a stacked pull request targeting the na-transfer branch.

Draft status

This PR intentionally remains a draft until MQT Core v4 is released. It does
not pin an intermediate Core commit and does not add temporary MLIR/CI setup.
CI is therefore expected to fail against the currently released Core version.

Dependencies

  • Depends on MQT Core #2249
    and the corresponding MQT Core v4 release.

Validation

Validated against the Core PR branch before removing the temporary pin:

  • 116/116 hybrid-map tests
  • focused serializer tests: 4/4
  • full uvx nox -s lint

AI assistance

Codex materially assisted with implementation, tests, integration validation,
and this pull request description. A human must review and understand the
changes before marking this pull request ready.

Checklist

  • The pull request only contains commits that are focused and relevant to this change.
  • I have added appropriate tests that cover the new/changed functionality.
  • I have updated the documentation to reflect these changes.
  • I have added entries to the changelog for any noteworthy additions, changes, fixes, or removals. The existing ♻️ Own neutral-atom stack #1111 entry covers this unreleased work.
  • I have added migration instructions to the upgrade guide (if needed).
  • The changes follow the project's style guidelines and introduce no new warnings.
  • The changes are fully tested and pass the CI checks. This waits for MQT Core v4.
  • I have reviewed my own code changes.

If PR contains AI-assisted content:

  • Any agent that created, edited, or submitted GitHub content was explicitly authorized for that scope, as required by our AI Usage Guidelines.
  • Every agent-authored or agent-edited public text body begins with the visible disclosure 🤖 *AI text below* 🤖 (titles are exempt).
  • I have disclosed AI assistance in the PR description.
  • I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

@github-project-automation github-project-automation Bot moved this to In Progress in MQT Compilation Aug 26, 2026
@simon1hofmann
simon1hofmann force-pushed the codex/2098-openqasm-serializer branch from f5750a8 to ede4c94 Compare August 26, 2026 11:14
@simon1hofmann simon1hofmann self-assigned this Aug 26, 2026
@simon1hofmann simon1hofmann added refactor Anything related to code refactoring c++ Anything related to C++ code dependencies Pull requests that update a dependency file labels Aug 26, 2026
@simon1hofmann
simon1hofmann marked this pull request as ready for review August 26, 2026 13:25

@denialhaag denialhaag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for also updating the implementation here, @simon1hofmann! 😎

Depending on how much we care about how clean the implementation is here, this could trigger some additional work in MQT Core. Otherwise, the OpenQASM-related changes largely look good to me. I'll try to get #1111 as soon as possible so that this PR isn't unnecessarily blocked.

Unrelated to that, I'm surprised to see all the MLIR-related changes in this PR. I don't think this should be necessary. 🤔

Comment thread cmake/ExternalDependencies.cmake
Comment thread cmake/ExternalDependencies.cmake Outdated
Comment thread include/hybridmap/AodOperation.hpp Outdated
Comment thread .github/workflows/ci.yml

@denialhaag denialhaag Aug 26, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess this is related to my previous comment, but I hope that we can do without setting up MLIR when v4 is released. 🤔

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@burgholzer, I lost track of how we want to handle this when v4 is released. Will all of the downstream repositories need MLIR as well? 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤖 AI text below 🤖

Agreed. The temporary MLIR workflow and packaging setup has been removed from the rewritten branch. This PR is intentionally a draft with expected CI failures until Core v4 is available.

Comment thread include/sc/DataLogger.hpp
Comment thread src/hybridmap/OpenQASMSerializer.cpp
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Important

Approval pending

CodeRabbit has no unresolved comments, but it has not reviewed the latest commit.

Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@simon1hofmann
simon1hofmann force-pushed the codex/2098-openqasm-serializer branch from 354dd87 to a2478c8 Compare August 26, 2026 14:38
@github-actions

github-actions Bot commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Cpp-Linter Report ⚠️

Some files did not pass the configured checks!

clang-tidy (v20.1.8) reports: 39 concern(s)
  • src/hybridmap/OpenQASMSerializer.cpp:16:10: error: [clang-diagnostic-error]

    'ir/OpenQASMSerializer.hpp' file not found

       16 | #include "ir/OpenQASMSerializer.hpp"
          |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~
  • src/hybridmap/OpenQASMSerializer.cpp:24:1: warning: [misc-include-cleaner]

    included header string is not used directly

       24 | #include <string>
          | ^~~~~~~~~~~~~~~~~
       25 | 
  • src/hybridmap/OpenQASMSerializer.cpp:34:15: warning: [misc-include-cleaner]

    no header providing "qc::QubitIndexToRegisterMap" is directly included

       18 |     const qc::QubitIndexToRegisterMap& qubitMap,
          |               ^
  • src/hybridmap/OpenQASMSerializer.cpp:35:32: warning: [misc-include-cleaner]

    no header providing "qc::BitIndexToRegisterMap" is directly included

       35 |     [[maybe_unused]] const qc::BitIndexToRegisterMap& bitMap,
          |                                ^
  • src/sc/DataLogger.cpp:15:1: warning: [misc-include-cleaner]

    included header OpType.hpp is not used directly

       15 | #include "ir/operations/OpType.hpp"
          | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
       16 | #include "sc/Architecture.hpp"
  • src/sc/DataLogger.cpp:18:1: warning: [misc-include-cleaner]

    included header utils.hpp is not used directly

       18 | #include "sc/utils.hpp"
          | ^~~~~~~~~~~~~~~~~~~~~~~
       19 | 
  • src/sc/DataLogger.cpp:20:1: warning: [misc-include-cleaner]

    included header array is not used directly

       20 | #include <array>
          | ^~~~~~~~~~~~~~~~
       21 | #include <cstddef>
  • src/sc/DataLogger.cpp:22:1: warning: [misc-include-cleaner]

    included header cstdint is not used directly

       22 | #include <cstdint>
          | ^~~~~~~~~~~~~~~~~~
       23 | #include <filesystem>
  • src/sc/DataLogger.cpp:26:1: warning: [misc-include-cleaner]

    included header limits is not used directly

       26 | #include <limits>
          | ^~~~~~~~~~~~~~~~~
       27 | #include <map>
  • src/sc/DataLogger.cpp:27:1: warning: [misc-include-cleaner]

    included header map is not used directly

       27 | #include <map>
          | ^~~~~~~~~~~~~~
       28 | #include <nlohmann/json.hpp>
  • src/sc/DataLogger.cpp:31:1: warning: [misc-include-cleaner]

    included header utility is not used directly

       31 | #include <utility>
          | ^~~~~~~~~~~~~~~~~~
       32 | #include <vector>
  • src/sc/DataLogger.cpp:90:20: warning: [cppcoreguidelines-init-variables]

    variable 'i' is not initialized

       90 |   for (std::size_t i = searchNodesLogFiles.size(); i <= layerIndex; ++i) {
          |                    ^                             
          |                                                   = 0
  • src/sc/DataLogger.cpp:132:3: warning: [misc-const-correctness]

    variable 'qasmStream' of type 'std::stringstream' (aka 'basic_stringstream') can be declared 'const'

      132 |   std::stringstream qasmStream;
          |   ^
          |                     const 
  • src/sc/DataLogger.cpp:139:5: warning: [misc-const-correctness]

    variable 'j' of type 'std::size_t' (aka 'unsigned long') can be declared 'const'

      139 |     std::size_t j = 0;
          |     ^
          |                 const 
  • src/sc/DataLogger.cpp:177:5: warning: [misc-const-correctness]

    variable 'i' of type 'std::size_t' (aka 'unsigned long') can be declared 'const'

      177 |     std::size_t i = 0;
          |     ^
          |                 const 
  • src/sc/DataLogger.cpp:194:21: warning: [cppcoreguidelines-init-variables]

    variable 'layerIndex' is not initialized

      194 |   const std::size_t layerIndex = searchNodesLogFiles.size() - 1;
          |                     ^                                          
          |                                                                 = 0
  • test/hybridmap/test_aodoperation.cpp:26:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       26 |   const na::AodOperation activate(na::NeutralAtomOperationKind::AodActivate,
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:35:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       35 |   const na::AodOperation deactivate(na::NeutralAtomOperationKind::AodDeactivate,
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:45:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       45 |   const na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0, 1},
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:56:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       56 |   const na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0, 1},
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:84:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       84 |   const na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0, 1},
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:86:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       86 |   const na::AodOperation move2("aod_move", {0, 1}, {dir1}, {0.0}, {1.0});
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:87:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       87 |   const na::AodOperation move3(na::NeutralAtomOperationKind::AodMove, {0},
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:90:26: error: [clang-diagnostic-error]

    variable type 'const na::AodOperation' is an abstract class

       90 |   const na::AodOperation move4(na::NeutralAtomOperationKind::AodMove, {0},
          |                          ^
  • test/hybridmap/test_aodoperation.cpp:97:3: warning: [misc-const-correctness]

    variable 'move' of type 'na::AodOperation' can be declared 'const'

       97 |   na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0},
          |   ^
          |                    const 
  • test/hybridmap/test_aodoperation.cpp:97:20: error: [clang-diagnostic-error]

    variable type 'na::AodOperation' is an abstract class

       97 |   na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0},
          |                    ^
  • test/hybridmap/test_aodoperation.cpp:106:3: warning: [misc-const-correctness]

    variable 'move' of type 'na::AodOperation' can be declared 'const'

      106 |   na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0},
          |   ^
          |                    const 
  • test/hybridmap/test_aodoperation.cpp:106:20: error: [clang-diagnostic-error]

    variable type 'na::AodOperation' is an abstract class

      106 |   na::AodOperation move(na::NeutralAtomOperationKind::AodMove, {0},
          |                    ^
  • test/hybridmap/test_aodoperation.cpp:112:3: warning: [misc-const-correctness]

    variable 'activate' of type 'na::AodOperation' can be declared 'const'

      112 |   na::AodOperation activate(na::NeutralAtomOperationKind::AodActivate, {0},
          |   ^
          |                    const 
  • test/hybridmap/test_aodoperation.cpp:112:20: error: [clang-diagnostic-error]

    variable type 'na::AodOperation' is an abstract class

      112 |   na::AodOperation activate(na::NeutralAtomOperationKind::AodActivate, {0},
          |                    ^
  • test/hybridmap/test_aodoperation.cpp:118:3: warning: [misc-const-correctness]

    variable 'deactivate' of type 'na::AodOperation' can be declared 'const'

      118 |   na::AodOperation deactivate(na::NeutralAtomOperationKind::AodDeactivate, {0},
          |   ^
          |                    const 
  • test/hybridmap/test_aodoperation.cpp:118:20: error: [clang-diagnostic-error]

    variable type 'na::AodOperation' is an abstract class

      118 |   na::AodOperation deactivate(na::NeutralAtomOperationKind::AodDeactivate, {0},
          |                    ^
  • test/hybridmap/test_neutral_atom_operation.cpp:31:3: warning: [misc-const-correctness]

    variable 'move' of type 'NeutralAtomOperation' can be declared 'const'

       31 |   NeutralAtomOperation move(NeutralAtomOperationKind::Move, {0, 1});
          |   ^
          |                        const 
  • test/hybridmap/test_neutral_atom_operation.cpp:31:24: error: [clang-diagnostic-error]

    variable type 'NeutralAtomOperation' is an abstract class

       31 |   NeutralAtomOperation move(NeutralAtomOperationKind::Move, {0, 1});
          |                        ^
    /home/runner/work/qmap/qmap/build/_deps/mqt-core-src/include/mqt-core/ir/operations/Operation.hpp:207:16: note: unimplemented pure virtual method 'dumpOpenQASM' in 'NeutralAtomOperation'
      207 |   virtual void dumpOpenQASM(std::ostream& of,
          |                ^
  • test/hybridmap/test_neutral_atom_operation.cpp:44:30: error: [clang-diagnostic-error]

    variable type 'const NeutralAtomOperation' is an abstract class

       44 |   const NeutralAtomOperation bridge(NeutralAtomOperationKind::Bridge,
          |                              ^
  • test/hybridmap/test_neutral_atom_operation.cpp:47:30: error: [clang-diagnostic-error]

    variable type 'const NeutralAtomOperation' is an abstract class

       47 |   const NeutralAtomOperation move(NeutralAtomOperationKind::Move, {0, 1, 2});
          |                              ^
  • test/hybridmap/test_neutral_atom_operation.cpp:138:22: error: [clang-diagnostic-error]

    variable type 'const AodOperation' is an abstract class

      138 |   const AodOperation first(NeutralAtomOperationKind::AodMove, {0},
          |                      ^
  • test/hybridmap/test_neutral_atom_operation.cpp:140:22: error: [clang-diagnostic-error]

    variable type 'const AodOperation' is an abstract class

      140 |   const AodOperation same(NeutralAtomOperationKind::AodMove, {0},
          |                      ^
  • test/hybridmap/test_neutral_atom_operation.cpp:142:22: error: [clang-diagnostic-error]

    variable type 'const AodOperation' is an abstract class

      142 |   const AodOperation different(NeutralAtomOperationKind::AodMove, {0},
          |                      ^

Have any feedback or feature suggestions? Share it here.

@denialhaag denialhaag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This LGTM now as well, and I'll leave the final say to @burgholzer! 😎

Edit: Well, I won't request a review from @burgholzer just yet. This will probably stay open for some weeks.

Comment thread include/hybridmap/AodOperation.hpp Outdated
Comment thread src/hybridmap/OpenQASMSerializer.cpp
Signed-off-by: Daniel Haag <121057143+denialhaag@users.noreply.github.com>

@denialhaag denialhaag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ahh, looks like the build is currently failing. 🤔

@burgholzer burgholzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This one was small enough to quickly browse through. Besides the CI errors flagged by Daniel, this looks pretty good to me.

@simon1hofmann

Copy link
Copy Markdown
Contributor Author

Ahh, looks like the build is currently failing. 🤔

Will keep failing until MQT Core v4 is released due to the missing header.

@denialhaag

Copy link
Copy Markdown
Member

Ahh, looks like the build is currently failing. 🤔

Will keep failing until MQT Core v4 is released due to the missing header.

That makes a lot of sense. I could have figured that out myself. 🧍

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 dependencies Pull requests that update a dependency file refactor Anything related to code refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants