Merge pull request #336 from saraedum/bylengthorder

Fix sorting of saddle connections in byLength() iteration
flatsurf · Jan 19, 2024 · 17938bc · 17938bc
2 parents edfbc0f + c8ecade
commit 17938bc
Show file tree

Hide file tree

Showing 4 changed files with 12 additions and 8 deletions.
diff --git a/doc/news/bylengthorder.rst b/doc/news/bylengthorder.rst
@@ -0,0 +1,3 @@
+**Fixed:**
+
+* Fixed ordering of saddle connections when iterating ``byLength()``.
diff --git a/libflatsurf/src/saddle_connections_by_length_iterator.cc b/libflatsurf/src/saddle_connections_by_length_iterator.cc
@@ -1,7 +1,7 @@
 /**********************************************************************
  *  This file is part of flatsurf.
  *
- *        Copyright (C) 2020 Julian Rüth
+ *        Copyright (C) 2020-2024 Julian Rüth
  *
  *  Flatsurf is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -91,7 +91,7 @@ void ImplementationOf<SaddleConnectionsByLengthIterator<Surface>>::increment() {
     auto withinBounds = connections.byAngle().bound(upperBoundInclusive).lowerBound(lowerBoundExclusive) | rx::to_vector();
     std::sort(begin(withinBounds), end(withinBounds), typename Vector<T>::CompareLength());
 
-    std::copy(rbegin(withinBounds), rend(withinBounds), std::back_inserter(connectionsWithinBounds));
+    std::copy(begin(withinBounds), end(withinBounds), std::back_inserter(connectionsWithinBounds));
   }
 }
 

diff --git a/libflatsurf/test/saddle_connections.test.cc b/libflatsurf/test/saddle_connections.test.cc
@@ -2,7 +2,7 @@
  *  This file is part of flatsurf.
  *
  *        Copyright (C)      2019 Vincent Delecroix
- *        Copyright (C) 2019-2022 Julian Rüth
+ *        Copyright (C) 2019-2024 Julian Rüth
  *
  *  Flatsurf is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -79,12 +79,13 @@ TEMPLATE_TEST_CASE("Saddle Connections on a Torus", "[SaddleConnections]", (long
   }
 
   SECTION("Saddle Connections Within a Fixed Bound Correspond to Coprime Coordinates") {
-    auto bound = GENERATE(0, 2, 16);
+    auto [bound_x, bound_y] = GENERATE(std::tuple{0, 0}, std::tuple{2, 0}, std::tuple{3, 1}, std::tuple{16, 0});
+    Bound bound(bound_x, bound_y);
 
     int expected = 0;
-    for (int x = 1; x < bound + 1; x++)
+    for (int x = 1; x < bound_x + bound_y + 1; x++)
       for (int y = 1; y <= x; y++)
-        if (x * x + y * y < bound * bound && std::gcd(x, y) == 1)
+        if (x * x + y * y <= bound.squared() && std::gcd(x, y) == 1)
           expected++;
 
     {
@@ -106,7 +107,7 @@ TEMPLATE_TEST_CASE("Saddle Connections on a Torus", "[SaddleConnections]", (long
     SECTION("We Find the Same Connections if we Iterate By Length") {
       auto count = 0;
       for (auto connection : square->connections().byLength()) {
-        if (connection > Bound(bound, 0))
+        if (connection > bound)
           break;
         count++;
       }

diff --git a/pyflatsurf/test/saddle_connections.py b/pyflatsurf/test/saddle_connections.py
@@ -83,6 +83,6 @@ def test_printing():
         assert str(connections) == "SaddleConnections()"
         connections = connections.byLength()
         assert str(connections) == "SaddleConnectionsByLength()"
-        assert str(next(iter(connections))) == "-2"
+        assert str(next(iter(connections))) == "1"
 
 if __name__ == '__main__': sys.exit(pytest.main(sys.argv))