Repository: adrn/gala (language: python)

Issue:
Flattened NFW never used? 
It seems that the builtin potential wrapper FlattenedNFWWrapper is not used when setting c!=1 in NFWPotential().

See example below
https://gist.github.com/balbinot/94d42b9ed1d9dc2e9ceffff2e1ac35fb

When calling NFW with c!=1 it seems to fall back to c=1 case, even when a and b are != 1

Using version 1.4.1, installed via conda (conda-forge) on Linux.




Discussion / hints:
Ack, thanks for the report! Will take a look ASAP.

Fix patch:
diff --git a/.github/workflows/wheels.yml b/.github/workflows/wheels.yml
index 9f37b0ef..17be8873 100644
--- a/.github/workflows/wheels.yml
+++ b/.github/workflows/wheels.yml
@@ -54,44 +54,6 @@ jobs:
         with:
           path: ./wheelhouse/*.whl
 
-  build_win_wheels:
-    name: Build ${{ matrix.python-version }} wheels on Windows
-    runs-on: windows-latest
-    if: github.event.pull_request.draft == false && !contains(github.event.pull_request.labels.*.name, 'docs only')
-    strategy:
-      matrix:
-        python-version: [3.7, 3.8] # , 3.9  # TODO
-      fail-fast: false
-
-    steps:
-      - uses: actions/checkout@v2
-        with:
-          fetch-depth: 0
-      - uses: conda-incubator/setup-miniconda@v2
-        with:
-          auto-update-conda: true
-          python-version: ${{ matrix.python-version }}
-          activate-environment: gala-wheels
-
-      - name: Install dependencies
-        shell: bash -l {0}
-        run: |
-          conda install -c conda-forge -q gsl libpython
-          gsl-config --version
-
-      - name: Build wheels
-        shell: bash -l {0}
-        run: |
-          which gsl-config
-          export GALA_GSL_VERSION=$(gsl-config --version)
-          export GALA_GSL_PREFIX="/c/Miniconda/envs/gala-wheels/Library/"
-          pip install pep517
-          python -m pep517.build -o wheelhouse .
-
-      - uses: actions/upload-artifact@v2
-        with:
-          path: ./wheelhouse/*.whl
-
   build_sdist:
     name: Build source distribution
     runs-on: ubuntu-latest
diff --git a/CHANGES.rst b/CHANGES.rst
index 4ffdc0f3..06169539 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -42,6 +42,10 @@ Bug fixes
   ``LeapfrogIntegrator`` that led to incorrect orbits for non-conservative
   systems that were integrated backwards (i.e. with ``dt<<0``).
 
+- Fixed a bug with the ``FlattenedNFW`` potential class in which the energy and
+  gradient functions were not using the inputted flattening (``c`` value) and
+  were instead defaulting to the spherical NFW model.
+
 API changes
 -----------
 
diff --git a/gala/potential/potential/builtin/builtin_potentials.c b/gala/potential/potential/builtin/builtin_potentials.c
index 3f820ef9..bfc00bb8 100644
--- a/gala/potential/potential/builtin/builtin_potentials.c
+++ b/gala/potential/potential/builtin/builtin_potentials.c
@@ -933,12 +933,14 @@ double flattenednfw_value(double t, double *pars, double *q, int n_dim) {
             - G (Gravitational constant)
             - m (scale mass)
             - r_s (scale radius)
-            - qz (flattening)
+            - a (ignore)
+            - b (ignore)
+            - c (z flattening)
     */
     double u, v_h2;
     // v_h2 = pars[1]*pars[1] / (log(2.) - 0.5);
     v_h2 = -pars[0] * pars[1] / pars[2];
-    u = sqrt(q[0]*q[0] + q[1]*q[1] + q[2]*q[2]/(pars[3]*pars[3])) / pars[2];
+    u = sqrt(q[0]*q[0] + q[1]*q[1] + q[2]*q[2]/(pars[5]*pars[5])) / pars[2];
     return v_h2 * log(1 + u) / u;
 }
 
@@ -947,18 +949,20 @@ void flattenednfw_gradient(double t, double *pars, double *q, int n_dim, double
             - G (Gravitational constant)
             - m (scale mass)
             - r_s (scale radius)
-            - qz (flattening)
+            - a (ignore)
+            - b (ignore)
+            - c (z flattening)
     */
     double fac, u, v_h2;
     // v_h2 = pars[1]*pars[1] / (log(2.) - 0.5);
     v_h2 = pars[0] * pars[1] / pars[2];
-    u = sqrt(q[0]*q[0] + q[1]*q[1] + q[2]*q[2]/(pars[3]*pars[3])) / pars[2];
+    u = sqrt(q[0]*q[0] + q[1]*q[1] + q[2]*q[2]/(pars[5]*pars[5])) / pars[2];
 
     fac = v_h2 / (u*u*u) / (pars[2]*pars[2]) * (log(1+u) - u/(1+u));
 
     grad[0] = grad[0] + fac*q[0];
     grad[1] = grad[1] + fac*q[1];
-    grad[2] = grad[2] + fac*q[2]/(pars[3]*pars[3]);
+    grad[2] = grad[2] + fac*q[2]/(pars[5]*pars[5]);
 }
 
 void flattenednfw_hessian(double t, double *pars, double *q, int n_dim, double *hess) {
@@ -966,12 +970,14 @@ void flattenednfw_hessian(double t, double *pars, double *q, int n_dim, double *
             - G (Gravitational constant)
             - m (mass scale)
             - r_s (scale radius)
-            - c (flattening)
+            - a (ignore)
+            - b (ignore)
+            - c (z flattening)
     */
     double G = pars[0];
     double m = pars[1];
     double r_s = pars[2];
-    double c = pars[3];
+    double c = pars[5];
     double x = q[0];
     double y = q[1];
     double z = q[2];


Test patch:
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 052c8351..d7a30fd0 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -50,10 +50,10 @@ jobs:
             python: 3.9
             toxenv: py39-test
 
-          - name: Python 3.9 standard tests (Windows)
-            os: windows-latest
-            python: 3.9
-            toxenv: py39-test
+          # - name: Python 3.9 standard tests (Windows)
+          #   os: windows-latest
+          #   python: 3.9
+          #   toxenv: py39-test
 
     steps:
       - name: Checkout code
@@ -76,25 +76,25 @@ jobs:
           python-version: ${{ matrix.python }}
 
       # Windows:
-      - uses: conda-incubator/setup-miniconda@v2
-        if: startsWith(matrix.os, 'windows')
-        with:
-          auto-update-conda: true
-          python-version: ${{ matrix.python-version }}
-
-      - name: Install Python dependencies - Windows
-        if: startsWith(matrix.os, 'windows')
-        shell: bash -l {0}
-        run: |
-          conda install -c conda-forge -q gsl python=3.9 libpython
-          python -m pip install -e .[test]
-          python -m pip install tox
-
-      - name: Run tests - Windows
-        if: startsWith(matrix.os, 'windows')
-        shell: bash -l {0}
-        run: |
-          tox ${{ matrix.toxargs }} -e ${{ matrix.toxenv }} ${{ matrix.toxposargs }}
+      # - uses: conda-incubator/setup-miniconda@v2
+      #   if: startsWith(matrix.os, 'windows')
+      #   with:
+      #     auto-update-conda: true
+      #     python-version: ${{ matrix.python-version }}
+
+      # - name: Install Python dependencies - Windows
+      #   if: startsWith(matrix.os, 'windows')
+      #   shell: bash -l {0}
+      #   run: |
+      #     conda install -c conda-forge -q gsl python=3.9 libpython
+      #     python -m pip install -e .[test]
+      #     python -m pip install tox
+
+      # - name: Run tests - Windows
+      #   if: startsWith(matrix.os, 'windows')
+      #   shell: bash -l {0}
+      #   run: |
+      #     tox ${{ matrix.toxargs }} -e ${{ matrix.toxenv }} ${{ matrix.toxposargs }}
 
       # Mac:
       - name: Setup Mac - GSL
diff --git a/gala/potential/potential/tests/test_all_builtin.py b/gala/potential/potential/tests/test_all_builtin.py
index 51dab931..0ed7b7c6 100644
--- a/gala/potential/potential/tests/test_all_builtin.py
+++ b/gala/potential/potential/tests/test_all_builtin.py
@@ -209,6 +209,21 @@ class TestSphericalNFW(PotentialTestBase):
     w0 = [19.0, 2.7, -6.9, 0.0352238, -0.03579493, 0.075]
 
 
+class TestFlattenedNFW(PotentialTestBase):
+    potential = p.NFWPotential(units=galactic, m=1E11, r_s=12., c=0.7)
+    w0 = [19.0, 2.7, -6.9, 0.0352238, -0.03579493, 0.075]
+    sympy_density = False  # not defined
+
+    def test_against_spherical(self):
+        """
+        Note: This is a regression test for Issue #254
+        """
+
+        sph = p.NFWPotential(units=galactic, m=1E11, r_s=12.)
+        assert not u.allclose(self.potential.gradient(self.w0[:3]),
+                              sph.gradient(self.w0[:3]))
+
+
 class TestTriaxialNFW(PotentialTestBase):
     potential = p.NFWPotential(units=galactic, m=1E11, r_s=12.,
                                a=1., b=0.95, c=0.9)