Repository: pace-neutrons/Euphonic (language: python)

Issue:
Missing Py_INCREF(Py_None) in the C extension
**Bug report**
I am trying to use Euphonic in a program that models and fits a phonon model to experimental data, which can require 10s of thousands of calls to `ForceConstants.calculate_qpoint_phonon_modes(...)`. At some point, the program crashes with a core dump and `Fatal Python error: none_dealloc: deallocating None`. 

**Bug fix**
I believe the issue is that the function `calculate_phonons(...)` of the C extension in `_euphonic.c` returns a `Py_None` (line 303 as of today), but the reference counting is not done as required. Consequently, Python tries at some point to deallocate/garbage collect `Py_None`, which is not allowed and causes the crash.

Incrementing the `Py_None` reference counter before returning it fixes the problem:
```
    Py_INCREF(Py_None);
    return Py_None;
```

For further details see, e.g., https://www.oreilly.com/library/view/python-cookbook/0596001673/ch16s05.html


Discussion / hints:

Fix patch:
diff --git a/.readthedocs.yml b/.readthedocs.yml
index 735d15763..7f938ece9 100644
--- a/.readthedocs.yml
+++ b/.readthedocs.yml
@@ -14,10 +14,10 @@ sphinx:
 python:
    version: 3.7
    install:
+      - requirements: doc/requirements.txt
       - method: pip
         path: .
         extra_requirements:
            - matplotlib
            - phonopy_reader
            - brille
-      - requirements: doc/requirements.txt
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 55ee0fa9a..913bb5c4b 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -5,6 +5,8 @@
 
   - Allow ``color`` to be passed as an extra kwarg to ``plot_1d`` and
     ``plot_1d_to_axis``. Previously this caused a ``TypeError``.
+  - Fix bug where ``Py_None`` was not incremented before returning from
+    ``calculate_phonons()`` in the C-extension causing a deallocation crash
 
 `v1.2.0 <https://github.com/pace-neutrons/Euphonic/compare/v1.1.0...v1.2.0>`_
 ------
diff --git a/c/_euphonic.c b/c/_euphonic.c
index 2a93b2f7e..84ce901f4 100644
--- a/c/_euphonic.c
+++ b/c/_euphonic.c
@@ -300,7 +300,7 @@ static PyObject *calculate_phonons(PyObject *self, PyObject *args) {
         Py_DECREF(py_dipole_init_data);
     }
 
-    return Py_None;
+    Py_RETURN_NONE;
 }
 
 static PyMethodDef _euphonic_methods[] = {
diff --git a/doc/requirements.txt b/doc/requirements.txt
index 6add1d498..8ddd5b313 100644
--- a/doc/requirements.txt
+++ b/doc/requirements.txt
@@ -1,3 +1,4 @@
+numpy>=1.14.5
 sphinx==5.3.0
 sphinx-argparse==0.3.2
 sphinx-autodoc-typehints==1.19.5


Test patch:
diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml
index 4596fba25..94cb3156b 100644
--- a/.github/workflows/run_tests.yml
+++ b/.github/workflows/run_tests.yml
@@ -73,7 +73,8 @@ jobs:
         run: |
           python -m pip install --upgrade pip
           python -m pip install -r tests_and_analysis/ci_requirements.txt
-          python -m pip install -r doc/requirements.txt numpy .[matplotlib,phonopy_reader,brille]
+          python -m pip install -r doc/requirements.txt
+          python -m pip install .[matplotlib,phonopy_reader,brille]
       - name: Run Sphinx doctests
         working-directory: ./doc
         shell: bash -l {0}