Merge pull request #292 from mj-will/fix-delta-phase-sign

Fix delta phase sign
mj-will · Mar 31, 2023 · d584945 · d584945
2 parents a555cb8 + 1b62f76
commit d584945
Showing 4 changed files with 32 additions and 14 deletions.
diff --git a/nessai/gw/reparameterisations.py b/nessai/gw/reparameterisations.py
@@ -181,7 +181,7 @@ def reparameterise(self, x, x_prime, log_j, **kwargs):
             Updated log Jacobian determinant
         """
         x_prime[self.prime_parameters[0]] = (
-            x[self.parameters[0]] - np.sign(np.cos(x["theta_jn"])) * x["psi"]
+            x[self.parameters[0]] + np.sign(np.cos(x["theta_jn"])) * x["psi"]
         )
         return x, x_prime, log_j
 
@@ -208,7 +208,7 @@ def inverse_reparameterise(self, x, x_prime, log_j, **kwargs):
         """
         x[self.parameters[0]] = np.mod(
             x_prime[self.prime_parameters[0]]
-            + np.sign(np.cos(x["theta_jn"])) * x["psi"],
+            - np.sign(np.cos(x["theta_jn"])) * x["psi"],
             2 * np.pi,
         )
         return x, x_prime, log_j

diff --git a/nessai/proposal/flowproposal.py b/nessai/proposal/flowproposal.py
@@ -658,7 +658,8 @@ def configure_reparameterisations(self, reparameterisations):
             r = rc(prior_bounds=prior_bounds, **default_config)
             self._reparameterisation.add_reparameterisations(r)
 
-        self.add_default_reparameterisations()
+        if self.use_default_reparameterisations:
+            self.add_default_reparameterisations()
 
         other_params = [
             n

diff --git a/tests/test_gw/test_gw_reparameterisations.py b/tests/test_gw/test_gw_reparameterisations.py
@@ -155,7 +155,7 @@ def test_delta_phase_reparameterise(delta_phase_reparam):
         delta_phase_reparam, x, x_prime, log_j
     )
     assert x_out == x
-    assert x_prime_out["delta_phase"] == 0.5
+    assert x_prime_out["delta_phase"] == 1.5
     assert log_j_out == 0
 
 
@@ -176,7 +176,7 @@ def test_delta_phase_inverse_reparameterise(delta_phase_reparam):
         delta_phase_reparam, x, x_prime, log_j
     )
     assert x_prime_out == x_prime
-    assert x["phase"] == 1.0
+    assert x["phase"] == 0.0
     assert log_j_out == 0
 
 
@@ -203,9 +203,15 @@ def test_delta_phase_inverse_invertible():
     x_prime["theta_jn"] = x["theta_jn"]
     log_j = np.zeros(n)
     x_f, x_prime_f, log_j_f = reparam.reparameterise(x, x_prime, log_j)
+
+    x_in = x_f.copy()
+    x_in["phase"] = np.nan
+
     assert_structured_arrays_equal(x_f, x)
     np.testing.assert_array_equal(log_j_f, log_j)
-    x_i, x_prime_i, log_j_i = reparam.reparameterise(x_f, x_prime_f, log_j_f)
+    x_i, x_prime_i, log_j_i = reparam.inverse_reparameterise(
+        x_in, x_prime_f.copy(), log_j_f.copy()
+    )
     assert_structured_arrays_equal(x_prime_i, x_prime_f)
-    assert_structured_arrays_equal(x_i, x)
+    assert_structured_arrays_equal(x_i, x, rtol=1e-15)
     np.testing.assert_array_equal(log_j_i, log_j_f)
diff --git a/tests/test_proposal/test_flowproposal/test_flowproposal_rescaling.py b/tests/test_proposal/test_flowproposal/test_flowproposal_rescaling.py
@@ -53,8 +53,13 @@ def test_get_reparamaterisation(mocked_fn, proposal):
 
 
 @pytest.mark.parametrize("reverse_order", [False, True])
+@pytest.mark.parametrize("use_default_reparameterisations", [False, True])
 def test_configure_reparameterisations_dict(
-    proposal, dummy_cmb_rc, dummy_rc, reverse_order
+    proposal,
+    dummy_cmb_rc,
+    dummy_rc,
+    reverse_order,
+    use_default_reparameterisations,
 ):
     """Test configuration for reparameterisations dictionary.
 
@@ -73,6 +78,7 @@ def test_configure_reparameterisations_dict(
     proposal.model.bounds = {"x": [-1, 1], "y": [-1, 1]}
     proposal.model.names = ["x"]
     proposal.reverse_reparameterisations = reverse_order
+    proposal.use_default_reparameterisations = use_default_reparameterisations
 
     with patch(
         "nessai.proposal.flowproposal.CombinedReparameterisation",
@@ -83,7 +89,12 @@ def test_configure_reparameterisations_dict(
         )
 
     proposal.get_reparameterisation.assert_called_once_with("default")
-    proposal.add_default_reparameterisations.assert_called_once()
+
+    if use_default_reparameterisations:
+        proposal.add_default_reparameterisations.assert_called_once()
+    else:
+        proposal.add_default_reparameterisations.assert_not_called()
+
     dummy_rc.assert_called_once_with(
         prior_bounds={"x": [-1, 1]}, parameters="x", boundary_inversion=True
     )
@@ -133,7 +144,7 @@ def test_configure_reparameterisations_dict_w_params(
         )
 
     proposal.get_reparameterisation.assert_called_once_with("default")
-    proposal.add_default_reparameterisations.assert_called_once()
+    proposal.add_default_reparameterisations.assert_not_called()
     dummy_rc.assert_called_once_with(
         prior_bounds={"x": [-1, 1], "y": [-1, 1]},
         parameters=["y", "x"],
@@ -213,7 +224,7 @@ def test_configure_reparameterisations_str(mocked_class, proposal):
     proposal.fallback_reparameterisation = None
     FlowProposal.configure_reparameterisations(proposal, {"x": "default"})
 
-    proposal.add_default_reparameterisations.assert_called_once()
+    proposal.add_default_reparameterisations.assert_not_called()
     assert proposal.rescaled_names == ["x_prime", "y"]
     assert proposal.rescale_parameters == ["x"]
     assert proposal._reparameterisation.parameters == ["x", "y"]
@@ -233,7 +244,7 @@ def test_configure_reparameterisations_dict_reparam(mocked_class, proposal):
         proposal, {"default": {"parameters": ["x"]}}
     )
 
-    proposal.add_default_reparameterisations.assert_called_once()
+    proposal.add_default_reparameterisations.assert_not_called()
     assert proposal.rescaled_names == ["x_prime", "y"]
     assert proposal.rescale_parameters == ["x"]
     assert proposal._reparameterisation.parameters == ["x", "y"]
@@ -250,7 +261,7 @@ def test_configure_reparameterisations_none(mocked_class, proposal):
     proposal.model.names = ["x", "y"]
     proposal.fallback_reparameterisation = None
     FlowProposal.configure_reparameterisations(proposal, None)
-    proposal.add_default_reparameterisations.assert_called_once()
+    proposal.add_default_reparameterisations.assert_not_called()
     assert proposal.rescaled_names == ["x", "y"]
 
     assert proposal.rescale_parameters == []
@@ -274,7 +285,7 @@ def test_configure_reparameterisations_fallback(mocked_class, proposal):
     proposal.model.names = ["x", "y"]
     proposal.fallback_reparameterisation = "default"
     FlowProposal.configure_reparameterisations(proposal, None)
-    proposal.add_default_reparameterisations.assert_called_once()
+    proposal.add_default_reparameterisations.assert_not_called()
     assert proposal.rescaled_names == ["x_prime", "y_prime"]
 
     assert proposal.rescale_parameters == ["x", "y"]