From bc5bc99fa6752313ae58da33cc9ea0acc9471625 Mon Sep 17 00:00:00 2001
From: Benjamin Lee <bnllee@ucdavis.edu>
Date: Sun, 8 Nov 2020 02:04:36 -0800
Subject: [PATCH] Fix helix position set on crossover for #488

---
 ...ces_positions_set_based_on_crossovers.dart | 26 +++++++++----------
 lib/src/state/helix.dart                      | 10 +++----
 test/helix_rotation_set_test.dart             | 24 ++++++++---------
 test/middleware_test.dart                     | 22 ++++++++--------
 4 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/lib/src/middleware/helices_positions_set_based_on_crossovers.dart b/lib/src/middleware/helices_positions_set_based_on_crossovers.dart
index 196ca287..e7dd8f7d 100644
--- a/lib/src/middleware/helices_positions_set_based_on_crossovers.dart
+++ b/lib/src/middleware/helices_positions_set_based_on_crossovers.dart
@@ -58,7 +58,7 @@ List<actions.UndoableAction> helix_positions_set_based_on_crossovers(AppState st
       continue;
     }
     double first_roll = helices[0].roll;
-    List<RollZY> rolls_and_positions =
+    List<RollXY> rolls_and_positions =
         _calculate_rolls_and_positions(state.design, helices, addresses, first_roll);
     var all_actions_this_group = set_rolls_and_positions(helices, rolls_and_positions);
     all_actions.addAll(all_actions_this_group);
@@ -262,41 +262,41 @@ Map<Tuple2<int, int>, List<Tuple2<Address, Address>>> _get_addresses_of_selected
 
 /// Represents a roll and 2D (x,y) position. The roll and position are both assigned to helices
 /// after gather what these should be by examining crossovers.
-class RollZY {
+class RollXY {
   double roll;
-  double z;
+  double x;
   double y;
 
-  RollZY({this.roll, this.z, this.y});
+  RollXY({this.roll, this.x, this.y});
 
-  String toString() => 'RollZY(roll=$roll, z=$z, y=$y)';
+  String toString() => 'RollZY(roll=$roll, x=$x, y=$y)';
 }
 
 //XXX: be careful editing this function; this logic was very tricky to get correct
 /// Return list of rolls, same length as [helices], giving the roll that each should be to point
 /// each pair of helix backbones at each other through the given [addresses].
 /// The first roll is [first_roll].
-List<RollZY> _calculate_rolls_and_positions(
+List<RollXY> _calculate_rolls_and_positions(
     Design design, List<Helix> helices, List<Tuple2<Address, Address>> addresses, double first_roll) {
   assert(helices.length == addresses.length + 1);
 
   Geometry geometry = design.geometry;
 
-  double x = helices[0].position3d().x;
+  double z = helices[0].position3d().z;
   double y = helices[0].position3d().y;
-  List<RollZY> rollxys = [RollZY(roll: first_roll, z: x, y: y)];
+  List<RollXY> rollxys = [RollXY(roll: first_roll, x: z, y: y)];
 
   for (int i = 0; i < addresses.length; i++) {
     var address_top = addresses[i].item1;
     var address_bot = addresses[i].item2;
     var roll = rollxys[i].roll;
-    var z = rollxys[i].z;
+    var x = rollxys[i].x;
     var y = rollxys[i].y;
 
     var degrees_top = design.helix_rotation_at(address_top, roll);
     // 0 is straight up, not right as in Cartesian rotation, so we have to convert
     var radians_top_cartesian = util.to_radians(degrees_top - 90);
-    var next_z = z + cos(radians_top_cartesian) * geometry.distance_between_helices_nm;
+    var next_x = x + cos(radians_top_cartesian) * geometry.distance_between_helices_nm;
     var next_y = y + sin(radians_top_cartesian) * geometry.distance_between_helices_nm;
 
     // now back to using our "0 is straight up" rotation coordinate system
@@ -316,19 +316,19 @@ List<RollZY> _calculate_rolls_and_positions(
     Helix helix_bot = helices[i + 1];
     // add this difference to the roll (which is defined at offset min_offset)
     var new_roll = (helix_bot.roll + delta_roll) % 360;
-    rollxys.add(RollZY(roll: new_roll, z: next_z, y: next_y));
+    rollxys.add(RollXY(roll: new_roll, x: next_x, y: next_y));
   }
 
   return rollxys;
 }
 
-List<actions.UndoableAction> set_rolls_and_positions(List<Helix> helices, List<RollZY> rolls_and_positions) {
+List<actions.UndoableAction> set_rolls_and_positions(List<Helix> helices, List<RollXY> rolls_and_positions) {
   List<actions.UndoableAction> all_actions = [];
   for (int i = 0; i < helices.length; i++) {
     var helix = helices[i];
     var rollxy = rolls_and_positions[i];
     var roll_action = actions.HelixRollSet(helix_idx: helix.idx, roll: rollxy.roll);
-    var position = Position3D(z: rollxy.z, y: rollxy.y, x: helix.position3d().x);
+    var position = Position3D(x: rollxy.x, y: rollxy.y, z: helix.position3d().z);
     var pos_action = actions.HelixPositionSet(helix_idx: helix.idx, position: position);
     all_actions.add(roll_action);
     all_actions.add(pos_action);
diff --git a/lib/src/state/helix.dart b/lib/src/state/helix.dart
index 10180ba2..be8132f5 100644
--- a/lib/src/state/helix.dart
+++ b/lib/src/state/helix.dart
@@ -169,7 +169,7 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H
 
   @memoized
   Position3D get default_position {
-    num x = min_offset * geometry.rise_per_base_pair;
+    num z = min_offset * geometry.rise_per_base_pair;
 
     // normalized so helices are diameter 1
     Point<num> point_zy;
@@ -184,7 +184,7 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H
     }
 
     num y = point_zy.y * geometry.distance_between_helices_nm;
-    num z = point_zy.x * geometry.distance_between_helices_nm;
+    num x = point_zy.x * geometry.distance_between_helices_nm;
     Position3D pos = Position3D(x: x, y: y, z: z);
     return pos;
   }
@@ -202,13 +202,13 @@ abstract class Helix with BuiltJsonSerializable, UnusedFields implements Built<H
     return default_position;
   }
 
-  /// Calculates z-y angle in degrees, according to position3d(), from this [Helix] to [other].
+  /// Calculates x-y angle in degrees, according to position3d(), from this [Helix] to [other].
   num angle_to(Helix other) {
     var pos1 = position3d();
     var pos2 = other.position3d();
-    num z = pos2.z - pos1.z;
+    num x = pos2.x - pos1.x;
     num y = pos2.y - pos1.y;
-    num angle_radians = (atan2(z, -y)) % (2 * pi); // using SVG "reverse y" coordinates
+    num angle_radians = (atan2(x, -y)) % (2 * pi); // using SVG "reverse y" coordinates
     return util.to_degrees(angle_radians);
   }
 
diff --git a/test/helix_rotation_set_test.dart b/test/helix_rotation_set_test.dart
index 2cf45b09..4fb3ca92 100644
--- a/test/helix_rotation_set_test.dart
+++ b/test/helix_rotation_set_test.dart
@@ -231,13 +231,13 @@ main() {
             idx: 1,
             grid: Grid.none,
             geometry: geometry,
-            position: Position3D(x: 0, z: helix_dist, y: helix_dist),
+            position: Position3D(x: helix_dist, z: 0, y: helix_dist),
             roll: 0),
         Helix(
             idx: 2,
             grid: Grid.none,
             geometry: geometry,
-            position: Position3D(x: 0, z: 2 * helix_dist, y: 2 * helix_dist),
+            position: Position3D(x: 2 * helix_dist, z: 0, y: 2 * helix_dist),
             roll: 0),
       ];
       design = Design(grid: Grid.none, helices: helices, geometry: geometry);
@@ -257,8 +257,8 @@ main() {
 
       num helix_dist = design.geometry.distance_between_helices_nm;
       expect(helix0.position, Position3D(x: 0, z: 0, y: 0));
-      expect(helix1.position, Position3D(x: 0, z: helix_dist, y: helix_dist));
-      expect(helix2.position, Position3D(x: 0, z: 2 * helix_dist, y: 2 * helix_dist));
+      expect(helix1.position, Position3D(x: helix_dist, z: 0, y: helix_dist));
+      expect(helix2.position, Position3D(x: 2 * helix_dist, z: 0, y: 2 * helix_dist));
 
       expect(util.rotation_between_helices(helix0, helix1, true, design.geometry), 90 + 45);
       expect(util.rotation_between_helices(helix1, helix2, true, design.geometry), 90 + 45);
@@ -272,11 +272,11 @@ main() {
 
     test('h0_h1_crossover_0_h1_h2_crossover_10', () {
       // with helix0.roll = 0, the forward backbone points up, so helix1 should be directly above
-      // y = -2.5, z = 0
+      // x = 0, y = -2.5
       // On helix1, the reverse backbone will point down, so the forward backbone will point 30 degrees
       // clockwise from straight up, which is 60 degrees counterclockwise from the x-axis in Cartesian.
       // So helix2 will be at
-      // y = -sin(60), z = cos(60)
+      // x = cos(60), y = -sin(60)
       List<actions.UndoableAction> all_actions = helix_positions_set_based_on_crossovers(state);
 
       expect(all_actions.length, 6);
@@ -293,18 +293,18 @@ main() {
       expect(helix0.position.y, closeTo(0, eps));
       expect(helix0.position.z, closeTo(0, eps));
 
-      num z1 = 0;
+      num x1 = 0;
       num y1 = -design.geometry.distance_between_helices_nm;
-      expect(helix1.position.x, closeTo(0, eps));
+      expect(helix1.position.x, closeTo(x1, eps));
       expect(helix1.position.y, closeTo(y1, eps));
-      expect(helix1.position.z, closeTo(z1, eps));
+      expect(helix1.position.z, closeTo(0, eps));
 
       num radians_60_deg = util.to_radians(60);
-      num z2 = cos(radians_60_deg) * design.geometry.distance_between_helices_nm;
+      num x2 = cos(radians_60_deg) * design.geometry.distance_between_helices_nm;
       num y2 = -sin(radians_60_deg) * design.geometry.distance_between_helices_nm;
-      expect(helix2.position.x, closeTo(0, eps));
+      expect(helix2.position.x, closeTo(x1 + x2, eps));
       expect(helix2.position.y, closeTo(y1 + y2, eps));
-      expect(helix2.position.z, closeTo(z1 + z2, eps));
+      expect(helix2.position.z, closeTo(0, eps));
     });
   });
 }
diff --git a/test/middleware_test.dart b/test/middleware_test.dart
index 62fb8cef..3fdd8540 100644
--- a/test/middleware_test.dart
+++ b/test/middleware_test.dart
@@ -213,67 +213,67 @@ main() {
             },
             {
               "roll": 330,
-              "position": {"x": 0, "y": 1.0847093477938932, "z": 2.2524221697560485},
+              "position": {"z": 0, "y": 1.0847093477938932, "x": 2.2524221697560485},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 300,
-              "position": {"x": 0, "y": 0.17135678687790956, "z": 4.5796065413665605},
+              "position": {"z": 0, "y": 0.17135678687790956, "x": 4.5796065413665605},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 270,
-              "position": {"x": 0, "y": -1.2369433582811473, "z": 6.645203477156547},
+              "position": {"z": 0, "y": -1.2369433582811473, "x": 6.645203477156547},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 240,
-              "position": {"x": 0, "y": -3.7090204238439677, "z": 7.017809142596987},
+              "position": {"z": 0, "y": -3.7090204238439677, "x": 7.017809142596987},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 210,
-              "position": {"x": 0, "y": -6.202029916796918, "z": 6.830983908630923},
+              "position": {"z": 0, "y": -6.202029916796918, "x": 6.830983908630923},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 180,
-              "position": {"x": 0, "y": -7.760754421443755, "z": 4.876405202460852},
+              "position": {"z": 0, "y": -7.760754421443755, "x": 4.876405202460852},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 150,
-              "position": {"x": 0, "y": -8.845463769237648, "z": 2.6239830327048033},
+              "position": {"z": 0, "y": -8.845463769237648, "x": 2.6239830327048033},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 120,
-              "position": {"x": 0, "y": -7.932111208321665, "z": 0.2967986610942912},
+              "position": {"z": 0, "y": -7.932111208321665, "x": 0.2967986610942912},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 90,
-              "position": {"x": 0, "y": -6.523811063162608, "z": -1.7687982746956945},
+              "position": {"z": 0, "y": -6.523811063162608, "x": -1.7687982746956945},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 60,
-              "position": {"x": 0, "y": -4.051733997599787, "z": -2.1414039401361347},
+              "position": {"z": 0, "y": -4.051733997599787, "x": -2.1414039401361347},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             },
             {
               "roll": 30,
-              "position": {"x": 0, "y": -1.558724504646837, "z": -1.9545787061700721},
+              "position": {"z": 0, "y": -1.558724504646837, "x": -1.9545787061700721},
               "major_ticks": [0, 10, 21, 31, 42, 52, 63],
               "max_offset": 64
             }