✨ Neutral Atom Mapper #417
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
include/na/Architecture.hpp
Outdated
| [[nodiscard]] auto length() { return std::abs(x * x + y * y); } | ||
| }; | ||
|
|
||
| // template <typename Value> using Grid = std::vector<std::vector<Value>>; |
Check notice
Code scanning / CodeQL
Commented-out code
include/na/Architecture.hpp
Outdated
| Value const tEff; | ||
| inline DecoherenceTimes(Value t1, Value t2) | ||
| : t1(t1), t2(t2), tEff(t1 * t2 / (t1 + t2)) {} | ||
| operator const double() const { return tEff; } |
Check warning
Code scanning / CodeQL
Constant return type on member
There was a problem hiding this comment.
Alright. Many thanks for this initial start. I took the time to go really in-depth with the comments here as this is one of the first reviews. Please do not feel overwhelmed by the amount of comments. Most of them are benign and very easy/fast to fix/address. Some are even just purely style guidelines that you should probably be aware of. but not even need to follow like a rule book.
If any of the comments should be unclear, I am sorry and please ask follow-up questions in the comments.
Some of the comments directly come with suggestions. In most cases, you can just apply these suggestions directly on GitHub. In case you are not aware of that: you can even go to the "Files Changes" Tab and add multiple suggestions to a batch. That way it results in fewer commits. Resolving suggestions this way also automatically resolves the corresponding comment.
If a comment does not warrant further discussion and you have implemented the requested changes or acknowledged the comment and there is no action item, feel free to resolve comments yourself. If a comment involves some back and forth, the last person commenting should typically not be the person resolving the comment.
Please let me know, if anything is unclear.
|
@burgholzer Thanks for the helpful review, this was exactly the degree of detail I was looking for. |
Co-authored-by: Lukas Burgholzer <burgholzer@me.com>
The Layer class constructs a DAG from the circuit that makes it efficient to maintain the set of executable gates.
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
Signed-off-by: burgholzer <burgholzer@me.com>
There was a problem hiding this comment.
Alright, made it through the whole code at least once in a rather quick fashion. Optimized a couple of things (mostly reserving the size for a couple of vectors and replacing the use of std::iota with explicit for loops).
To be fair, I did not go fully in depth, when it comes to the larger functions in the Mapper itself, as these are rather convoluted. But from quickly glancing over everything, it seems reasonable. We can always iterate on the code itself later on and improve things.
What I noticed is that some of the docstrings are a bit messed up/incomplete (especially noticed in the NAGraphAlgorithms file). Maybe worth looking into before eventually merging this PR here.
Once you are satisfied with everything, I'd say we can go ahead with merging the mqt-core PR, update the submodule here and get this PR merged. 🎉
## Description This pull request includes all modifications and data structures that are generally useful and required for the implementation of the neutral atom mapper, see [mqt-qmap#417](cda-tum/mqt-qmap#417). ## Checklist: <!--- This checklist serves as a reminder of a couple of things that ensure your pull request will be merged swiftly. --> - [x] The pull request only contains commits that are related to it. - [x] I have added appropriate tests and documentation. - [x] I have made sure that all CI jobs on GitHub pass. - [x] The pull request introduces no new warnings and follows the project's style guidelines.
Description
This is the start of a mapper tailored to the neutral atom platform. Peculiarities are global gates and shuttling of atoms, among others.
Checklist: