✅ Refine NALAC tests (#470)

## Description

This PR adds proper tests (and fixes revealed bugs) for the Neutral Atom
Logical Array Compiler (NALAC).

## 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.

---------

Signed-off-by: burgholzer <burgholzer@me.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: burgholzer <burgholzer@me.com>

include/hybridmap/NeutralAtomArchitecture.hpp

@@ -382,9 +382,9 @@ class NeutralAtomArchitecture {
    * @param idx2 The index of the second coordinate
    * @return The Euclidean distance between the two coordinate indices
    */
-  [[nodiscard]] qc::fp getEuclidianDistance(CoordIndex idx1,
-                                            CoordIndex idx2) const {
-    return static_cast<qc::fp>(this->coordinates.at(idx1).getEuclidianDistance(
+  [[nodiscard]] qc::fp getEuclideanDistance(const CoordIndex idx1,
+                                            const CoordIndex idx2) const {
+    return static_cast<qc::fp>(this->coordinates.at(idx1).getEuclideanDistance(
         this->coordinates.at(idx2)));
   }
   /**
@@ -393,9 +393,9 @@ class NeutralAtomArchitecture {
    * @param c2 The second coordinate
    * @return The Euclidean distance between the two coordinates
    */
-  [[nodiscard]] static qc::fp getEuclidianDistance(const Point& c1,
+  [[nodiscard]] static qc::fp getEuclideanDistance(const Point& c1,
                                                    const Point& c2) {
-    return static_cast<qc::fp>(c1.getEuclidianDistance(c2));
+    return static_cast<qc::fp>(c1.getEuclideanDistance(c2));
   }
   /**
    * @brief Get the Manhattan distance between two coordinate indices

noxfile.py

@@ -85,7 +85,6 @@ def minimums(session: nox.Session) -> None:
         session,
         install_args=["--resolution=lowest-direct"],
         run_args=["-Wdefault"],
-        extras=["qiskit", "evaluation"],
     )
     session.run("uv", "pip", "list")
 

src/hybridmap/HardwareQubits.cpp

@@ -134,7 +134,7 @@ void HardwareQubits::computeNearbyQubits(HwQubit q) {
     if (coord.first == q) {
       continue;
     }
-    if (arch->getEuclidianDistance(coordQ, coord.second) <=
+    if (arch->getEuclideanDistance(coordQ, coord.second) <=
         arch->getInteractionRadius()) {
       newNearbyQubits.emplace(coord.first);
     }
@@ -169,7 +169,7 @@ HardwareQubits::getBlockedQubits(const std::set<HwQubit>& qubits) {
         continue;
       }
       // TODO improve by using the nearby coords as a preselection
-      auto const distance = arch->getEuclidianDistance(hwToCoordIdx.at(qubit),
+      auto const distance = arch->getEuclideanDistance(hwToCoordIdx.at(qubit),
                                                        hwToCoordIdx.at(i));
       if (distance <=
           arch->getBlockingFactor() * arch->getInteractionRadius()) {

src/hybridmap/HybridNeutralAtomMapper.cpp

@@ -846,7 +846,7 @@ qc::fp NeutralAtomMapper::moveCostPerLayer(const AtomMove& move,
             continue;
           }
           auto hwQubit = this->mapping.getHwQubit(qubit);
-          auto dist    = this->arch.getEuclidianDistance(
+          auto dist    = this->arch.getEuclideanDistance(
               this->hardwareQubits.getCoordIndex(hwQubit),
               this->hardwareQubits.getCoordIndex(toMoveHwQubit));
           distanceBefore += dist;
@@ -857,7 +857,7 @@ qc::fp NeutralAtomMapper::moveCostPerLayer(const AtomMove& move,
             continue;
           }
           auto hwQubit = this->mapping.getHwQubit(qubit);
-          auto dist    = this->arch.getEuclidianDistance(
+          auto dist    = this->arch.getEuclideanDistance(
               this->hardwareQubits.getCoordIndex(hwQubit), move.second);
           distanceAfter += dist;
         }

src/hybridmap/NeutralAtomArchitecture.cpp

@@ -136,7 +136,7 @@ void NeutralAtomArchitecture::computeSwapDistances(qc::fp interactionRadius) {
 
   for (uint32_t i = 0; i < this->getNcolumns() && i < interactionRadius; i++) {
     for (uint32_t j = i; j < this->getNrows(); j++) {
-      auto const dist = NeutralAtomArchitecture::getEuclidianDistance(
+      auto const dist = NeutralAtomArchitecture::getEuclideanDistance(
           Point(0, 0), Point(i, j));
       if (dist <= interactionRadius) {
         if (dist == 0) {
@@ -266,7 +266,7 @@ NeutralAtomArchitecture::getBlockedCoordIndices(const qc::Operation* op) const {
       }
       // do a preselection
       // now check exact difference
-      auto const distance = getEuclidianDistance(coord, i);
+      auto const distance = getEuclideanDistance(coord, i);
       if (distance <= getBlockingFactor() * getInteractionRadius()) {
         blockedCoordIndices.emplace(i);
       }

src/na/NAMapper.cpp

@@ -42,23 +42,23 @@ namespace na {
 
 auto NAMapper::validateCircuit() -> void {
   for (const auto& op : initialQc) {
-    if (op->isCompoundOperation() and isGlobal(*op, initialQc.getNqubits())) {
+    if (op->isCompoundOperation() && isGlobal(*op, initialQc.getNqubits())) {
       const auto& co = dynamic_cast<qc::CompoundOperation&>(*op);
       if (!arch.isAllowedGlobally({co.at(0)->getType(), 0})) {
         std::stringstream ss;
         ss << "The chosen architecture does not support the operation "
            << FullOpType{op->getType(), 0} << " globally.";
         throw std::invalid_argument(ss.str());
       }
-    } else if (op->isStandardOperation() and op->isSingleQubitGate()) {
+    } else if (op->isStandardOperation() && op->isSingleQubitGate()) {
       assert(op->getNcontrols() == 0);
       if (!arch.isAllowedLocally({op->getType(), 0})) {
         std::stringstream ss;
         ss << "The chosen architecture does not support the operation "
            << FullOpType{op->getType(), 0} << " locally.";
         throw std::invalid_argument(ss.str());
       }
-    } else if (op->isStandardOperation() and
+    } else if (op->isStandardOperation() &&
                op->getNcontrols() + op->getNtargets() == 2) {
       assert(!op->isSingleQubitGate());
       if (!arch.isAllowedLocally({op->getType(), op->getNcontrols()})) {
@@ -433,8 +433,8 @@ auto NAMapper::store(std::vector<bool>&             initialFreeSites,
   }
   std::sort(freeSitesPerRow.begin(), freeSitesPerRow.end(),
             [&](const auto& a, const auto& b) {
-              return (std::get<1>(a) == std::get<1>(b) and
-                      std::get<0>(a) < std::get<0>(b)) or
+              return (std::get<1>(a) == std::get<1>(b) &&
+                      std::get<0>(a) < std::get<0>(b)) ||
                      std::get<1>(a) > std::get<1>(b);
             });
   std::size_t moveableSpotsNeeded = qubits.size();
@@ -446,7 +446,7 @@ auto NAMapper::store(std::vector<bool>&             initialFreeSites,
       if (n != std::get<1>(freeSitesPerRow.at(firstIWithSameN))) {
         firstIWithSameN = i;
       }
-      if (i + 1 == freeSitesPerRow.size() or
+      if (i + 1 == freeSitesPerRow.size() ||
           std::get<1>(freeSitesPerRow[i + 1]) < moveableSpotsNeeded) {
         const auto [rF, nF] = freeSitesPerRow.at(firstIWithSameN);
         moveableSelectedRows.emplace_back(rF, moveableSpotsNeeded);
@@ -493,7 +493,7 @@ auto NAMapper::store(std::vector<bool>&             initialFreeSites,
           currentFreeSites.at(site) = false;
           initialFreeSites.at(site) = false;
           n -= 1;
-        } else if (j < sitesInRow.size() and
+        } else if (j < sitesInRow.size() &&
                    currentFreeSites.at(sitesInRow.at(j))) {
           const auto& s    = sitesInRow.at(j);
           const auto& sPos = arch.getPositionOfSite(s);
@@ -585,7 +585,7 @@ auto NAMapper::pickUp(std::vector<bool>&             initialFreeSites,
                               [&](const auto& acc, const auto& s) {
                                 return acc + (initialFreeSites.at(s) ? 1 : 0);
                               });
-          if (freeSpotsInRow <= spotsNeeded and n > freeSpotsInRow) {
+          if (freeSpotsInRow <= spotsNeeded && n > freeSpotsInRow) {
             freeSpotsInRow = n;
             zone           = z;
             row            = r;
@@ -642,7 +642,7 @@ auto NAMapper::pickUp(std::vector<bool>&             initialFreeSites,
           // check whether j can be
           // picked up
           if (placement.at(p).positionStatus == Atom::PositionStatus::DEFINED) {
-            if (placement.at(p).currentPosition->y == y and
+            if (placement.at(p).currentPosition->y == y &&
                 placement.at(p).currentPosition->x <= x - d) {
               // pick up p
               pickUpOrder.erase(
@@ -673,7 +673,7 @@ auto NAMapper::pickUp(std::vector<bool>&             initialFreeSites,
               }
               const auto site = *siteOpt;
               freeX           = arch.getPositionOfSite(site).x;
-              if (initialFreeSites.at(site) and
+              if (initialFreeSites.at(site) &&
                   std::find(placement.at(p).zones.cbegin(),
                             placement.at(p).zones.cend(),
                             arch.getZoneOfSite(site)) !=
@@ -730,7 +730,7 @@ auto NAMapper::pickUp(std::vector<bool>&             initialFreeSites,
       } else {
         // check whether j can be picked up
         if (placement.at(p).positionStatus == Atom::PositionStatus::DEFINED) {
-          if (placement.at(p).currentPosition->y == y and
+          if (placement.at(p).currentPosition->y == y &&
               placement.at(p).currentPosition->x >= x - d) {
             // pick up p
             pickUpOrder.erase(
@@ -760,7 +760,7 @@ auto NAMapper::pickUp(std::vector<bool>&             initialFreeSites,
             }
             const auto site = *siteOpt;
             freeX           = arch.getPositionOfSite(site).x;
-            if (initialFreeSites.at(site) and
+            if (initialFreeSites.at(site) &&
                 std::find(placement.at(p).zones.cbegin(),
                           placement.at(p).zones.cend(),
                           arch.getZoneOfSite(site)) !=
@@ -852,7 +852,7 @@ auto NAMapper::map(const qc::QuantumComputation& qc) -> void {
           const auto* const co = dynamic_cast<const qc::CompoundOperation*>(op);
           mappedQc.emplaceBack<NAGlobalOperation>(
               FullOpType{co->at(0)->getType(), 0}, co->at(0)->getParameter());
-        } else if (isGlobal(*op, nqubits) and
+        } else if (isGlobal(*op, nqubits) &&
                    arch.isAllowedGlobally(
                        {op->getType(), op->getNcontrols()})) {
           mappedQc.emplaceBack<NAGlobalOperation>(
@@ -865,8 +865,12 @@ auto NAMapper::map(const qc::QuantumComputation& qc) -> void {
           for (const auto& v :
                layer.getExecutablesOfType(op->getType(), op->getNcontrols())) {
             const auto* const op2 = v->getOperation();
-            if (checkApplicability(op2, placement) and
-                op->getParameter() == op2->getParameter()) {
+            if (checkApplicability(op2, placement) &&
+                op->getParameter() == op2->getParameter() &&
+                std::find(
+                    positions.cbegin(), positions.cend(),
+                    placement.at(op2->getTargets().front()).currentPosition) ==
+                    positions.cend()) {
               updatePlacement(op2, placement);
               v->execute();
               positions.emplace_back(
@@ -890,7 +894,7 @@ auto NAMapper::map(const qc::QuantumComputation& qc) -> void {
     //    of the same type and two targets, i.e. cz gates
     if (config.getMethod() == NAMappingMethod::Naive) {
       const qc::Operation* op = (*it)->getOperation();
-      if (op->getType() != qc::OpType::Z or op->getNtargets() != 1 or
+      if (op->getType() != qc::OpType::Z || op->getNtargets() != 1 ||
           op->getNcontrols() != 1) {
         throw std::logic_error(
             "Other gates than cz are not supported for mapping yet.");
@@ -1055,7 +1059,7 @@ auto NAMapper::map(const qc::QuantumComputation& qc) -> void {
             ss << "Atom " << q << " was unexpectedly not picked up.";
             throw std::logic_error(ss.str());
           }
-          if (x >= 0 and static_cast<std::size_t>(x) < sites.size()) {
+          if (x >= 0 && static_cast<std::size_t>(x) < sites.size()) {
             const auto pos =
                 arch.getPositionOfSite(sites.at(static_cast<std::size_t>(x)));
             placement.at(q).currentPosition =
@@ -1097,8 +1101,14 @@ auto NAMapper::map(const qc::QuantumComputation& qc) -> void {
             moveableOrdered, storageZone);
       // -------------------------------------------------------------
       std::vector<qc::Qubit> fixedVector;
-      std::transform(fixed.cbegin(), fixed.cend(), back_inserter(fixedVector),
+      fixedVector.reserve(fixed.size());
+      std::transform(fixed.cbegin(), fixed.cend(),
+                     std::back_inserter(fixedVector),
                      [](const auto& v) { return v.first; });
+      std::sort(fixedVector.begin(), fixedVector.end(),
+                [&fixed](const auto& a, const auto& b) {
+                  return fixed.at(a) < fixed.at(b);
+                });
       store(initialFreeSites, currentFreeSites, placement, currentlyShuttling,
             fixedVector, storageZone);
     } else {
@@ -1135,8 +1145,8 @@ auto NAMapper::map(const qc::QuantumComputation& qc) -> void {
   stats.numInitialGates    = qc.getNops();
   stats.numEntanglingGates = static_cast<std::size_t>(
       std::count_if(qc.cbegin(), qc.cend(), [](const auto& op) {
-        return (op->getType() == qc::OpType::Z and
-                op->getType() == qc::OpType::X) or
+        return (op->getType() == qc::OpType::Z &&
+                op->getType() == qc::OpType::X) ||
                op->getNcontrols() > 0;
       }));
   stats.initialDepth   = qc.getDepth();

test/hybridmap/test_hybridmap.cpp

@@ -40,7 +40,7 @@ TEST_P(NeutralAtomArchitectureTest, LoadArchitecures) {
   EXPECT_GE(arch.getGateTime("cz"), 0);
   EXPECT_GE(arch.getGateAverageFidelity("cz"), 0);
   // Test distance functions
-  EXPECT_GE(arch.getEuclidianDistance(c1, c2), 0);
+  EXPECT_GE(arch.getEuclideanDistance(c1, c2), 0);
   // Test MoveVector functions
   auto mv = arch.getVector(0, 1);
   EXPECT_GE(arch.getVectorShuttlingTime(mv), 0);