🐛 Bug fixes and improvements related to the coordinate system

[wlambooy]

Description

This PR is a collection of bug fixes in order to make the experience working with layouts of different coordinate types more seamless. In particular, the following changes are introduced:

• All coordinate types now have a wrap function added to their respective namespaces that takes a dimension coordinate. When the coordinate is beyond the given dimension, it wraps to the next coordinate within the dimension. If no such coordinate exists, it becomes a dead copy of the dimension coordinate.
• Coordinate iteration now does not go into an infinite loop when an out of bounds iterator is given.
• Area and volume computation is now dependent on the coordinate type
• Cartesian layouts with SiQAD coordinates now always represent full dimer rows; this means that the z dimension is always 1.
• An integral_abs function defined on integral number types was added to math_utils.hpp. Simply using std::abs could give an ambiguity error or a compiler warning when applied to an unsigned integer. This function handles these concerns such that it can be applied to any integral number type without compiler errors or warnings.
• Addition and subtraction is now defined properly on SiQAD coordinates.
• static_assert checks were added to apply_gate_layout and relative_to_absolute_cell_position, making sure that the CellLyt template parameter is not based in SiQAD coordinates.
• Some changes to the docs: the siqad namespace in coordinates.hpp had missing docs; the "Offset coordinates are required" line was removed from cartesian_layout.hpp.
• In layout_utils.hpp, converting between SiQAD and fiction coordinates now appropriately resizes the layout. Note that since layouts with SiQAD coordinates must always have full dimer rows, the area of the transformed layout is not always equal to the area of the original layout when converting to SiQAD coordinates. The tests have thus been updated accordingly.
• The print_charge_layout function was adapted to the new changes. Also layout cropping is now optional (default false).

Tests were added for all of the new behaviour. The wrap function implementations are tested through the coordinate iterator tests.

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.

[codecov]

Codecov Report

Merging #225 (d5ba024) into main (08792fe) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #225      +/-   ##
==========================================
+ Coverage   94.41%   94.44%   +0.03%     
==========================================
  Files          82       82              
  Lines        7436     7477      +41     
==========================================
+ Hits         7021     7062      +41     
  Misses        415      415              

Impacted Files	Coverage Δ
.../algorithms/physical_design/apply_gate_library.hpp	100.00% <ø> (ø)
include/fiction/io/print_layout.hpp	94.96% <100.00%> (-0.18%)	⬇️
include/fiction/layouts/cartesian_layout.hpp	98.30% <100.00%> (+0.04%)	⬆️
include/fiction/layouts/coordinates.hpp	99.53% <100.00%> (+0.09%)	⬆️
include/fiction/layouts/hexagonal_layout.hpp	98.50% <100.00%> (ø)
include/fiction/utils/layout_utils.hpp	97.64% <100.00%> (ø)
include/fiction/utils/math_utils.hpp	100.00% <100.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08792fe...d5ba024. Read the comment docs.

[marcelwa]

Many thanks for these great additions and fixes. Looks very good overall. You have really improved the code stability! Just a couple of minor comments from my side here.

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[wlambooy]

It seems that some compilers generate a warning when the result of the new abs function is casted to an integer. Apparently they do not take the type traits check for integrality into account. I will drop the support for floating point values from this function as this can be achieved without ambiguity using std::fabs, making the intended use to be able to take the abs of any integral type number without compiler warnings.

EDIT: The latest commit fixed this issue and no extra warnings are generated

[wlambooy]

Windows CI build is out of heap space again 🙄

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[marcelwa]

Looks essentially done. I just spotted a small inconsistency. After this, it's done, I promise 😆

[github-actions]

clang-tidy review says "All clean, LGTM! 👍"

[marcelwa]

Many thanks!