♻️ Refactor FoMaC classes by MatthiasReumann · Pull Request #1849 · munich-quantum-toolkit/core

MatthiasReumann · 2026-07-03T12:13:45Z

Description

Proposed FoMaC rewrite:

Use unique_ptr instead of custom destructor, copy, and move semantics.
Remove trailing return types as the return type does not depend on the arguments.
Remove "Tokens". Use private constructors and friendship semantics.
Consistency improvements (std::size_t vs size_t, etc.)

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.
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.
I have reviewed my own code changes.

If PR contains AI-assisted content:

I have disclosed the use of AI tools in the PR description as per our AI Usage Guidelines.
AI-assisted commits include an Assisted-by: [Model Name] via [Tool Name] footer.
I confirm that I have personally reviewed and understood all AI-generated content, and accept full responsibility for it.

codecov · 2026-07-03T12:19:11Z

Codecov Report

❌ Patch coverage is 93.66516% with 14 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
include/mqt-core/fomac/FoMaC.hpp	90.9%	10 Missing ⚠️
src/fomac/FoMaC.cpp	96.0%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!

…m-toolkit/core into enh/refactor-fomac

burgholzer

This generally looks good to me and we can happily move forward with this, if you @MatthiasReumann feel confident in the changes.

I have two very small comments and a medium one that we could also tackle after this PR to not stall it for too long.

In any case, this still needs labels and a changelog entry.
We should also backport these changes to the v3.x series so that the changes can go into the next release there.

@denialhaag for awareness.

burgholzer · 2026-07-06T14:35:38Z

+  }
+
+  /// @returns the underlying QDMI_Device object.
+  [[nodiscard]] QDMI_Device getQDMIDevice() const { return device_; }


Would getHandle potentially be a better name? one could also generically call these private members handle_ because that's what they essentially are. The above method would/could then also become fromHandle.

Yes, I like that 👍🏻

Because you mention fromHandle (or fromQDMIDevice): Could you maybe elaborate a bit on what the rationale of the Tokens was? Because as far as I can tell fromQDMIDevice short-circuits these guardrails anyway (only to make the Python bindings work...).
A question that I've asked myself is: Would it be that bad to be able to construct a "Site" class from a site QDMI pointer?

Interestingly, I just re-googled the definition of "handle". Apparently, a "handle" is the object holding the raw pointer (basically our FoMaC classes!). So ptr_ and fromPointer would probably be semantically even more correct. Analogously to unique_ptr the method get could then return the underlying pointer.

Let me know what you think!

Yes, I like that 👍🏻

Because you mention fromHandle (or fromQDMIDevice): Could you maybe elaborate a bit on what the rationale of the Tokens was? Because as far as I can tell fromQDMIDevice short-circuits these guardrails anyway (only to make the Python bindings work...). A question that I've asked myself is: Would it be that bad to be able to construct a "Site" class from a site QDMI pointer?

I am on that recovering my thoughts about the Tokens. I do remember that I also considered friend classes but deliberately decided against them. Give me some minutes to recover my rationale there.

I'll answer in the main thread of this PR as I deem it as fundamental for this PR.

ystade · 2026-07-07T06:58:02Z

@MatthiasReumann Continuing the discussion in #1849 (comment):

The initial motivation of the FoMaC implementation was to mirror the QDMI behavior on a C++ level. This meant not only to provide C++ wrappers around the C handles but also to mirror the access pattern, i.e., aSite can only be retrieved from a Device or an Operation or a Device can only be retrieved from an active Session with a Driver. As you, @MatthiasReumann correctly mentioned, from QDMIdevice short-circuits this intended behavior.

To implement the described behavior, I decided to use Tokens instead of using friend classes to not expose all other private functions to the outside world and have a more fine-granular way to steer what can be accessed from outside the class.

The AI also mentioned that the Python bindings do not understand friendship. However, I do not recall, how this influenced the decision but maybe a point to check/keep in mind.

burgholzer · 2026-07-07T08:55:03Z

@MatthiasReumann Continuing the discussion in #1849 (comment):

The initial motivation of the FoMaC implementation was to mirror the QDMI behavior on a C++ level. This meant not only to provide C++ wrappers around the C handles but also to mirror the access pattern, i.e., aSite can only be retrieved from a Device or an Operation or a Device can only be retrieved from an active Session with a Driver. As you, @MatthiasReumann correctly mentioned, from QDMIdevice short-circuits this intended behavior.

To implement the described behavior, I decided to use Tokens instead of using friend classes to not expose all other private functions to the outside world and have a more fine-granular way to steer what can be accessed from outside the class.

The AI also mentioned that the Python bindings do not understand friendship. However, I do not recall, how this influenced the decision but maybe a point to check/keep in mind.

The reason for the token solution was exactly to avoid friend semantics. I am (personally) not the biggest fan of them because it always feels like bad design to me.
However, it feels like the solution here works just fine 🙂 and it feels moderately clean.

Regarding the overall abstraction: I think we could allow ourselves some freedom and do not necessarily stick to the C design. However, one has to be careful that most of the opaque pointers (site, operation, job, etc.) are tied to a particular session with a device.
So passing around the handles without the corresponding "owner" is dangerous.

MatthiasReumann · 2026-07-07T10:08:45Z

I am (personally) not the biggest fan of them because it always feels like bad design to me.

I think there is only one way to use friendships correctly in C++: A friend class A constructs an object B with private constructor.

So passing around the handles without the corresponding "owner" is dangerous.

This is what bugs me about the Python bindings using the fromQDMIDevice function.

burgholzer · 2026-07-07T10:26:34Z

I am (personally) not the biggest fan of them because it always feels like bad design to me.

I think there is only one way to use friendships correctly in C++: A friend class A constructs an object B with private constructor.

Yeah. This feels fine 👍🏻

So passing around the handles without the corresponding "owner" is dangerous.

This is what bugs me about the Python bindings using the fromQDMIDevice function.

Yeah. There are a couple more things that bother me about certain ergonomics at the moment. Most importantly the way we inject external QDMI device libraries at runtime, which circumvents the QDMI client interface. But it was the only way to pass authentication information to the actual device.

MatthiasReumann · 2026-07-07T10:40:29Z

@burgholzer @ystade

Okay. Maybe let's summarize the discussion and action items up to this point to get this current PR merged.

⚡️I'll think a bit about the QDMI C structure in C++ (or FoMaC classes). Particularly, the ownership-semantics and change the implementation, if necessary.
💬 You discuss QInfos. A follow-up pull request will then deal with the custom property getters.
💭 We'll leave the Python Bindings as they are for now.

Sounds good?

burgholzer · 2026-07-07T10:42:09Z

@burgholzer @ystade

Okay. Maybe let's summarize the discussion and action items up to this point to get this current PR merged.

⚡️I'll think a bit about the QDMI C structure in C++ (or FoMaC classes). Particularly, the ownership-semantics and change the implementation, if necessary.

💬 You discuss QInfos. A follow-up pull request will then deal with the custom property getters.

💭 We'll leave the Python Bindings as they are for now.

Sounds good?

Sounds good to me 👍🏻
Leaving both the bindings as well as the custom properties to follow up PRs is reasonable because these do not need to block the PR here.

MatthiasReumann and others added 7 commits July 3, 2026 09:54

Update FoMaC

96ecaa0

Move sites also

ebbf057

Minor reformatting

12094c5

Implement token behavior via friendship and private constructors

e596d32

Update bindings

ef0c7df

Remove newline

ed47c8b

🎨 pre-commit fixes

dc757db

MatthiasReumann added 3 commits July 3, 2026 14:19

Merge branch 'main' into enh/refactor-fomac

3c21605

Fix linting

43ad17e

Merge branch 'enh/refactor-fomac' of https://github.com/munich-quantu…

24bf95c

…m-toolkit/core into enh/refactor-fomac

MatthiasReumann mentioned this pull request Jul 3, 2026

🚧 Integrate QDMI Devices #1687

Draft

11 tasks

🎨 pre-commit fixes

c6991f4

burgholzer requested changes Jul 6, 2026

View reviewed changes

MatthiasReumann added code quality Code quality improvements c++ Anything related to C++ code QDMI Anything related to QDMI labels Jul 7, 2026

MatthiasReumann added 3 commits July 7, 2026 07:47

Add starship operator to Job class

507b810

Try to fix lint

ce56b35

Merge branch 'main' into enh/refactor-fomac

25c561f

MatthiasReumann self-assigned this Jul 7, 2026

Use Device object pointers

235c76b

coderabbitai Bot approved these changes Jul 7, 2026

View reviewed changes

burgholzer reviewed Jul 7, 2026

View reviewed changes

Comment thread include/mqt-core/fomac/FoMaC.hpp Outdated

Use implicit conversions

fc13a1e

Uh oh!

Uh oh!

Conversation

MatthiasReumann commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

codecov Bot commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

burgholzer left a comment

Choose a reason for hiding this comment

Uh oh!

burgholzer Jul 6, 2026

Choose a reason for hiding this comment

Uh oh!

MatthiasReumann Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatthiasReumann Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ystade Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

ystade Jul 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ystade commented Jul 7, 2026

Uh oh!

burgholzer commented Jul 7, 2026

Uh oh!

MatthiasReumann commented Jul 7, 2026

Uh oh!

burgholzer commented Jul 7, 2026

Uh oh!

MatthiasReumann commented Jul 7, 2026

Uh oh!

burgholzer commented Jul 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MatthiasReumann commented Jul 3, 2026 •

edited

Loading

codecov Bot commented Jul 3, 2026 •

edited

Loading

MatthiasReumann Jul 7, 2026 •

edited

Loading

MatthiasReumann Jul 7, 2026 •

edited

Loading