Address PR comments

docs/source/user_manual/search_space.rst

@@ -78,8 +78,8 @@ The ``VelocityGridSearch`` strategy searches a uniform grid of x and y velocitie
 | ``max_vy``             | The maximum velocity in the y-dimension (pixels per day). |
 +------------------------+-----------------------------------------------------------+
 
-EclipticSearch
---------------
+EclipticCenteredSearch
+----------------------
 
 The grid is defined by two sets of parameters: a sampling of absolute velocities in pixels per day and a sampling of the velocities' angles in degrees or radians. Each sampling consists of values defining the range and number of sampling steps. 
 
@@ -95,37 +95,32 @@ Given the linear sampling for both velocities and angles, the full set of candid
 
 where ``angles`` contains the list of angles to test and ``velocities`` contains the list of velocities. 
 
-The list of velocities is created from the given bounds (``min_vel`` and ``max_vel``) and number of steps (``vel_steps``). The range is inclusive of both bounds.
+The list of velocities is created from the given bounds list ``velocities=[min_vel, max_vel, vel_steps]``. The range is inclusive of both bounds.
 
 Each angle in the list is computed as an **offset** from the ecliptic angle. KBMOD uses the following ordering for extracting the ecliptic.
-1. If ``force_ecliptic`` is provided (is not ``None``) in the generator’s configuration that value is used directly.
+1. If ``given_ecliptic`` is provided (is not ``None``) in the generator’s configuration that value is used directly.
 2. If the first image has a WCS, the ecliptic is estimated from that WCS.
 3. A default ecliptic of 0.0 is used.
-The list of angles spans ``[ecliptic + min_ang_offset, ecliptic + max_ang_offset]`` inclusive of both bounds. Angles can be specified in degrees or radians (as noted by the ``angle_units`` parameter) but must be consistent among all angles.
+The list of angles used is defined from the list ``angles=[min_offset, max_offset, angle_steps]`` and will span ``[ecliptic + min_offset, ecliptic + max_offset]`` inclusive of both bounds. Angles can be specified in degrees or radians (as noted by the ``angle_units`` parameter) but must be consistent among all angles.
 
 
 +------------------------+-----------------------------------------------------+
 | **Parameter**          | **Interpretation**                                  |
 +------------------------+-----------------------------------------------------+
-| ``ang_steps``          | The number of angle steps.                          |
+| ``angles``            | A length 3 tuple with the minimum angle offset,     |
+|                        | the maximum offset, and the number of angles to     |
+|                        | to search through (angles specified in either       |
+|                        | radians or degrees)                                 |
 +------------------------+-----------------------------------------------------+
 | ``angle_units``        | The units to use for angles.                        |
 |                        | Either 'degrees' or 'radians'                       |
 +------------------------+-----------------------------------------------------+
-| ``min_ang_offset ``    | The minimum angular offset from the ecliptic        |
-|                        | (in either radians or degrees).                     |
-+------------------------+-----------------------------------------------------+
-| ``max_ang_offset ``    | The maximum angular offset from the ecliptic        |
+| ``given_ecliptic ``    | The given value of the ecliptic angle               |
 |                        | (in either radians or degrees).                     |
 +------------------------+-----------------------------------------------------+
-| ``force_ecliptic ``    | The given value of the ecliptic angle               |
-|                        | (in either radians or degrees).                     |
-+------------------------+-----------------------------------------------------+
-| ``vel_steps ``         | The number of velocity steps.                       |
-+------------------------+-----------------------------------------------------+
-| ``min_vel ``           | The minimum velocity magnitude (in pixels per day). |
-+------------------------+-----------------------------------------------------+
-| ``max_vel ``           | The maximum velocity magnitude (in pixels per day). |
+| ``velocities ``        | A length 3 tuple with the minimum velocity (in      |
+|                        | pixels per day), the maximum velocity (in pixels    |
+|                        | per day), and number of velocities to test.         |
 +------------------------+-----------------------------------------------------+
 
 

notebooks/KBMOD_Demo.ipynb

@@ -125,13 +125,9 @@
     "    # Use search parameters (including a force ecliptic angle of 0.0)\n",
     "    # to match what we know is in the demo data.\n",
     "    \"generator_config\": {\n",
-    "        \"name\": \"EclipticSearch\",\n",
-    "        \"vel_steps\": 21,\n",
-    "        \"min_vel\": 0.0,\n",
-    "        \"max_vel\": 20.0,\n",
-    "        \"ang_steps\": 11,\n",
-    "        \"min_ang_offset\": -0.5,\n",
-    "        \"max_ang_offset\": 0.5,\n",
+    "        \"name\": \"EclipticCenteredSearch\",\n",
+    "        \"angles\": [-0.5, 0.5, 11],\n",
+    "        \"velocities\": [0.0, 20.0, 21],\n",
     "        \"angle_units\": \"radians\",\n",
     "        \"force_ecliptic\": 0.0,\n",
     "    },\n",

notebooks/create_fake_data.ipynb

@@ -150,13 +150,9 @@
     "settings = {\n",
     "    # Override the search data to match the known object.\n",
     "    \"generator_config\": {\n",
-    "        \"name\": \"EclipticSearch\",\n",
-    "        \"vel_steps\": 21,\n",
-    "        \"min_vel\": 0.0,\n",
-    "        \"max_vel\": 20.0,\n",
-    "        \"ang_steps\": 11,\n",
-    "        \"min_ang_offset\": -0.5,\n",
-    "        \"max_ang_offset\": 0.5,\n",
+    "        \"name\": \"EclipticCenteredSearch\",\n",
+    "        \"angles\": [-0.5, 0.5, 11],\n",
+    "        \"velocities\": [0.0, 20.0, 21],\n",
     "        \"angle_units\": \"radians\",\n",
     "        \"force_ecliptic\": 0.0,\n",
     "    },\n",

src/kbmod/configuration.py

@@ -32,15 +32,11 @@ def __init__(self):
             "do_stamp_filter": True,
             "encode_num_bytes": -1,
             "generator_config": {
-                "name": "EclipticSearch",
-                "vel_steps": 257,
-                "min_vel": 92.0,
-                "max_vel": 526.0,
-                "ang_steps": 129,
-                "min_ang_offset": -math.pi / 15,
-                "max_ang_offset": math.pi / 15,
+                "name": "EclipticCenteredSearch",
+                "velocity": [92.0, 526.0, 257],
+                "angles": [-math.pi / 15, math.pi / 15, 129],
                 "angle_units": "radians",
-                "force_ecliptic": None,
+                "given_ecliptic": None,
             },
             "gpu_filter": False,
             "ind_output_files": True,

src/kbmod/fake_data/demo_helper.py

@@ -39,15 +39,11 @@ def make_demo_data(filename=None):
     settings = {
         # Override the search data to match the known object.
         "generator_config": {
-            "name": "EclipticSearch",
-            "vel_steps": 21,
-            "min_vel": 0.0,
-            "max_vel": 20.0,
-            "ang_steps": 11,
-            "min_ang_offset": -0.5,
-            "max_ang_offset": 0.5,
+            "name": "EclipticCenteredSearch",
+            "velocities": [0, 20.0, 21],
+            "angles": [-0.5, 0.5, 11],
             "angle_units": "radians",
-            "force_ecliptic": 0.0,
+            "given_ecliptic": 0.0,
         },
         # Loosen the other filtering parameters.
         "clip_negative": True,

src/kbmod/trajectory_generator.py

@@ -334,25 +334,19 @@ def __init__(self, v_arr, ang_arr, average_angle=None, computed_ecliptic_angle=N
         super().__init__(v_arr[2], v_arr[0], v_arr[1], ang_arr[2], ang_min, ang_max, **kwargs)
 
 
-class EclipticSearch(TrajectoryGenerator):
+class EclipticCenteredSearch(TrajectoryGenerator):
     """Search a grid defined by velocities and angles relative to the ecliptic.
 
     Attributes
     ----------
     ecliptic_angle : `float`
         The angle to use for the ecliptic in the image (in the units defined in ``angle_units``).
-    vel_steps : `int`
-        The number of velocity steps.
-    min_vel : `float`
-        The minimum velocity magnitude (in pixels per day)
-    max_vel : `float`
-        The maximum velocity magnitude (in pixels per day)
-    ang_steps : `int`
-        The number of angle steps.
-    min_ang_offset : `float`
-        The minimum offset of the search angle relative from the ecliptic_angle (in radians).
-    max_ang_offset : `float`
-        The maximum offset of the search angle relative from the ecliptic_angle (in radians).
+    velocities : `list`
+        A triplet of the minimum velocity to use (in pixels per day), and the maximum velocity
+        magnitude (in pixels per day), and the number of velocity steps to try.
+    angles : `list`
+        A triplet of the minimum angle offset (in radians), and the maximum angle offset (in radians),
+        and the number of angles to try.
     min_ang : `float`
         The minimum search angle in the image (ecliptic_angle + min_ang_offset) in radians.
     max_ang : `float`
@@ -361,39 +355,27 @@ class EclipticSearch(TrajectoryGenerator):
 
     def __init__(
         self,
-        vel_steps=0,
-        min_vel=0.0,
-        max_vel=0.0,
-        ang_steps=0,
-        min_ang_offset=0.0,
-        max_ang_offset=0.0,
+        velocities=[0.0, 0.0, 0],
+        angles=[0.0, 0.0, 0],
         angle_units="radians",
-        force_ecliptic=None,
+        given_ecliptic=None,
         computed_ecliptic_angle=None,
         **kwargs,
     ):
-        """Create a class KBMODV1Search.
+        """Create a class EclipticCenteredSearch.
 
         Parameters
         ----------
-        vel_steps : `int`
-            The number of velocity steps.
-        min_vel : `float`
-            The minimum velocity magnitude (in pixels per day)
-        max_vel : `float`
-            The maximum velocity magnitude (in pixels per day)
-        ang_steps : `int`
-            The number of angle steps.
-        min_ang_offset : `float`
-            The minimum offset of the search angle relative from the ecliptic_angle
-            (in the units defined in ``angle_units``).
-        max_ang_offset : `float`
-            The maximum offset of the search angle relative from the ecliptic_angle
-            (in the units defined in ``angle_units``).
+        velocities : `list`
+            A triplet of the minimum velocity to use (in pixels per day), and the maximum velocity
+            magnitude (in pixels per day), and the number of velocity steps to try.
+        angles : `list`
+            A triplet of the minimum angle offset (in the units defined in ``angle_units``), the maximum
+            angle offset (in the units defined in ``angle_units``), and the number of angles to try.
         angle_units : `str`
             The units for the angle. Must be one of radians or degrees.
             Default: 'radians'
-        force_ecliptic : `float`, optional
+        given_ecliptic : `float`, optional
             An override for the ecliptic as given in the config (in the units defined in
             ``angle_units``). This angle takes precedence over ``computed_ecliptic``.
         computed_ecliptic_angle : `float`, optional
@@ -402,57 +384,52 @@ def __init__(
         """
         super().__init__(**kwargs)
 
-        if force_ecliptic is not None:
-            ecliptic_angle = force_ecliptic
+        if given_ecliptic is not None:
+            ecliptic_angle = given_ecliptic
         elif computed_ecliptic_angle is not None:
             ecliptic_angle = computed_ecliptic_angle
         else:
             logger = logging.getLogger(__name__)
             logger.warning("No ecliptic angle provided. Using 0.0.")
             ecliptic_angle = 0.0
 
-        if vel_steps < 1 or ang_steps < 1:
-            raise ValueError("EclipticSearch requires at least 1 step in each dimension")
-        if max_vel < min_vel:
-            raise ValueError("Invalid EclipticSearch bounds.")
+        if velocities[2] < 1 or angles[2] < 1:
+            raise ValueError("EclipticCenteredSearch requires at least 1 step in each dimension")
+        if velocities[1] < velocities[0]:
+            raise ValueError(f"Invalid EclipticCenteredSearch bounds: {velocities}")
 
-        self.vel_steps = vel_steps
-        self.min_vel = min_vel
-        self.max_vel = max_vel
-        self.vel_stepsize = (self.max_vel - self.min_vel) / float(self.vel_steps - 1)
+        self.velocities = velocities
+        self.vel_stepsize = (velocities[1] - velocities[0]) / float(velocities[2] - 1)
 
         # Convert the angles into radians.
-        if angle_units[:3] == "rad":
-            self.ecliptic_angle = ecliptic_angle
-            self.min_ang_offset = min_ang_offset
-            self.max_ang_offset = max_ang_offset
-        elif angle_units[:3] == "deg":
+        self.angles = angles
+        self.ecliptic_angle = ecliptic_angle
+        if angle_units[:3] == "deg":
             deg_to_rad = math.pi / 180.0
-            self.ecliptic_angle = deg_to_rad * ecliptic_angle
-            self.min_ang_offset = deg_to_rad * min_ang_offset
-            self.max_ang_offset = deg_to_rad * max_ang_offset
-        else:
+            self.ecliptic_angle = deg_to_rad * self.ecliptic_angle
+            self.angles[0] = deg_to_rad * self.angles[0]
+            self.angles[1] = deg_to_rad * self.angles[1]
+        elif angle_units[:3] != "rad":
             raise ValueError(f"Unknown angular units {angle_units}")
 
-        self.ang_steps = ang_steps
-        self.min_ang = self.ecliptic_angle + self.min_ang_offset
-        self.max_ang = self.ecliptic_angle + self.max_ang_offset
-        self.ang_stepsize = (self.max_ang - self.min_ang) / float(self.ang_steps - 1)
+        self.min_ang = self.ecliptic_angle + self.angles[0]
+        self.max_ang = self.ecliptic_angle + self.angles[1]
+        self.ang_stepsize = (self.max_ang - self.min_ang) / float(self.angles[2] - 1)
 
     def __repr__(self):
         return (
             "EclipticSearch:"
-            f" v=[{self.min_vel}, {self.max_vel}], {self.vel_steps}"
+            f" v=[{self.velocities[0]}, {self.velocities[1]}], {self.velocities[2]}"
             f" a=[{self.min_ang}, {self.max_ang}], {self.ang_steps}"
         )
 
     def __str__(self):
         return f"""EclipticSearch:
-               Vel: [{self.min_vel}, {self.max_vel}] in {self.vel_steps} steps.
+               Vel: [{self.velocities[0]}, {self.velocities[1]}] in {self.velocities[2]} steps.
                Ang:
                    Ecliptic = {self.ecliptic_angle}
-                   Offsets = {self.min_ang_offset} to {self.max_ang_offset}
-                   [{self.min_ang}, {self.max_ang}] in {self.ang_steps} steps."""
+                   Offsets = {self.angles[0]} to {self.angles[1]}
+                   [{self.min_ang}, {self.max_ang}] in {self.angles[2]} steps."""
 
     def generate(self, *args, **kwargs):
         """Produces a single candidate trajectory to test.
@@ -462,10 +439,10 @@ def generate(self, *args, **kwargs):
         candidate : `Trajectory`
             A ``Trajectory`` to test at each pixel.
         """
-        for ang_i in range(self.ang_steps):
-            for vel_i in range(self.vel_steps):
+        for ang_i in range(self.angles[2]):
+            for vel_i in range(self.velocities[2]):
                 curr_ang = self.min_ang + ang_i * self.ang_stepsize
-                curr_vel = self.min_vel + vel_i * self.vel_stepsize
+                curr_vel = self.velocities[0] + vel_i * self.vel_stepsize
 
                 vx = math.cos(curr_ang) * curr_vel
                 vy = math.sin(curr_ang) * curr_vel

tests/test_configuration.py

@@ -52,18 +52,20 @@ def test_from_hdu(self):
                 ["Here3"],
                 ["7"],
                 ["null"],
-                [
-                    "[1, 2]",
-                ],
+                ["[1, 2]"],
+                ["{name: test_gen, p1: [1.0, 2.0], p2: 2.0}"],
             ],
-            names=("im_filepath", "num_obs", "cluster_type", "ang_arr"),
+            names=("im_filepath", "num_obs", "cluster_type", "ang_arr", "generator_config"),
         )
         hdu = fits.table_to_hdu(t)
 
         config = SearchConfiguration.from_hdu(hdu)
         self.assertEqual(config["im_filepath"], "Here3")
         self.assertEqual(config["num_obs"], 7)
         self.assertEqual(config["ang_arr"], [1, 2])
+        self.assertEqual(config["generator_config"]["name"], "test_gen")
+        self.assertEqual(config["generator_config"]["p1"], [1.0, 2.0])
+        self.assertEqual(config["generator_config"]["p2"], 2.0)
         self.assertIsNone(config["cluster_type"])
 
     def test_to_hdu(self):
@@ -73,7 +75,7 @@ def test_to_hdu(self):
             "cluster_type": None,
             "do_clustering": False,
             "res_filepath": "There",
-            "generator_config": {"name": "test_gen", "p1": 1.0, "p2": 2.0},
+            "generator_config": {"name": "test_gen", "p1": [1.0, 2.0], "p2": 2.0},
             "basic_array": [1.0, 2.0, 3.0],
         }
         config = SearchConfiguration.from_dict(d)
@@ -83,7 +85,7 @@ def test_to_hdu(self):
         self.assertEqual(hdu.data["num_obs"][0], "5\n...")
         self.assertEqual(hdu.data["cluster_type"][0], "null\n...")
         self.assertEqual(hdu.data["res_filepath"][0], "There\n...")
-        self.assertEqual(hdu.data["generator_config"][0], "{name: test_gen, p1: 1.0, p2: 2.0}")
+        self.assertEqual(hdu.data["generator_config"][0], "{name: test_gen, p1: [1.0, 2.0], p2: 2.0}")
         self.assertEqual(hdu.data["basic_array"][0], "[1.0, 2.0, 3.0]")
 
     def test_to_yaml(self):
@@ -93,7 +95,7 @@ def test_to_yaml(self):
             "cluster_type": None,
             "do_clustering": False,
             "res_filepath": "There",
-            "generator_config": {"name": "test_gen", "p1": 1.0, "p2": 2.0},
+            "generator_config": {"name": "test_gen", "p1": [1.0, 2.0], "p2": 2.0},
         }
         config = SearchConfiguration.from_dict(d)
         yaml_str = config.to_yaml()
@@ -104,7 +106,7 @@ def test_to_yaml(self):
         self.assertEqual(yaml_dict["cluster_type"], None)
         self.assertEqual(yaml_dict["res_filepath"], "There")
         self.assertEqual(yaml_dict["generator_config"]["name"], "test_gen")
-        self.assertEqual(yaml_dict["generator_config"]["p1"], 1.0)
+        self.assertEqual(yaml_dict["generator_config"]["p1"], [1.0, 2.0])
         self.assertEqual(yaml_dict["generator_config"]["p2"], 2.0)
 
     def test_save_and_load_yaml(self):

tests/test_regression_test.py

@@ -245,16 +245,12 @@ def perform_search(im_stack, res_filename, default_psf):
         "psf_val": default_psf,
         "output_suffix": "",
         "generator_config": {
-            "name": "EclipticSearch",
-            "vel_steps": 52,
-            "min_vel": 92.0,
-            "max_vel": 550.0,
-            "ang_steps": 26,
+            "name": "EclipticCenteredSearch",
             # Offset by PI for prograde orbits in lori allen data
-            "min_ang_offset": np.pi - np.pi / 10.0,
-            "max_ang_offset": np.pi + np.pi / 10.0,
+            "angles": [np.pi - np.pi / 10.0, np.pi + np.pi / 10.0, 26],
+            "velocities": [92.0, 550.0, 52],
             "angle_units": "radians",
-            "force_ecliptic": 1.1901106654050821,
+            "given_ecliptic": 1.1901106654050821,
         },
         "num_obs": 15,
         "do_mask": True,