ARM-DOE / pyart

The Python-ARM Radar Toolkit. A data model driven interactive toolkit for working with weather radar data.
https://arm-doe.github.io/pyart/
Other
513 stars 266 forks source link

Migrate to Cython 3.0 #1439

Closed mgrover1 closed 1 year ago

mgrover1 commented 1 year ago

Description

Cython recently released version 3.0, which is leading to breaking CI for Py-ART due to some API changes + updated needed on our side.

Example failing CI

Installing build dependencies: started
  Installing build dependencies: finished with status 'done'
  Checking if build backend supports build_editable: started
  Checking if build backend supports build_editable: finished with status 'done'
  Getting requirements to build editable: started
  Getting requirements to build editable: finished with status 'error'
  error: subprocess-exited-with-error

  × Getting requirements to build editable did not run successfully.
  │ exit code: 1
  ╰─> [63 lines of output]

      Error compiling Cython file:
      ------------------------------------------------------------
      ...
                  r += dabs(x[i]-y[i])
                  if r>upperbound:
                      return r
          else:
              for i in range(k):
                  r += dabs(x[i]-y[i])**p
                  ^
      ------------------------------------------------------------

      pyart/map/ckdtree.pyx:318:12: Cannot assign type 'npy_float64 complex' to 'float64_t'
      Compiling pyart/__check_build/_check_build.pyx because it changed.
      Compiling pyart/correct/_fast_edge_finder.pyx because it changed.
      Compiling pyart/correct/_unwrap_1d.pyx because it changed.
      Compiling pyart/correct/_unwrap_2d.pyx because it changed.
      Compiling pyart/correct/_unwrap_3d.pyx because it changed.
      Compiling pyart/io/_sigmetfile.pyx because it changed.
      Compiling pyart/io/nexrad_interpolate.pyx because it changed.
      Compiling pyart/map/ckdtree.pyx because it changed.
      Compiling pyart/map/_load_nn_field_data.pyx because it changed.
      Compiling pyart/map/_gate_to_grid_map.pyx because it changed.
      Compiling pyart/retrieve/_kdp_proc.pyx because it changed.
      [ 1/11] Cythonizing pyart/__check_build/_check_build.pyx
      [ 2/11] Cythonizing pyart/correct/_fast_edge_finder.pyx
      [ 3/11] Cythonizing pyart/correct/_unwrap_1d.pyx
      [ 4/11] Cythonizing pyart/correct/_unwrap_2d.pyx
      [ 5/11] Cythonizing pyart/correct/_unwrap_3d.pyx
      [ 6/11] Cythonizing pyart/io/_sigmetfile.pyx
      [ 7/11] Cythonizing pyart/io/nexrad_interpolate.pyx
      [ 8/11] Cythonizing pyart/map/_gate_to_grid_map.pyx
      [ 9/11] Cythonizing pyart/map/_load_nn_field_data.pyx
      [[10](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:11)/[11](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:12)] Cythonizing pyart/map/ckdtree.pyx
      Traceback (most recent call last):
        File "/Users/runner/micromamba/envs/pyart-dev/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
        File "/Users/runner/micromamba/envs/pyart-dev/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/Users/runner/micromamba/envs/pyart-dev/lib/python3.11/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line [13](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:14)2, in get_requires_for_build_editable
          return hook(config_settings)
                 ^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 450, in get_requires_for_build_editable
          return self.get_requires_for_build_wheel(config_settings)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 341, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=['wheel'])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 3[23](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:24), in _get_build_requires
          self.run_setup()
        File "/private/var/folders/[24](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:25)/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 488, in run_setup
          self).run_setup(setup_script=setup_script)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 338, in run_setup
          exec(code, locals())
        File "<string>", line [27](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:28)6, in <module>
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/Cython/Build/Dependencies.py", line 1134, in cythonize
          cythonize_one(*args)
        File "/private/var/folders/24/8k48jl6d249_n_qfxwsl6xvm0000gn/T/pip-build-env-8x2jk9ac/overlay/lib/python3.11/site-packages/Cython/Build/Dependencies.py", line 1[30](https://github.com/ARM-DOE/pyart/actions/runs/5585457918/jobs/10208219378#step:5:31)1, in cythonize_one
          raise CompileError(None, pyx_file)
      Cython.Compiler.Errors.CompileError: pyart/map/ckdtree.pyx
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build editable did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

Proposed solution

We should pin to cython < 3.0 for now, and work on this migration.

wolfidan commented 1 year ago

Hi @mgrover1 ,

A dirty quick fix I used in our Py-ART fork is to replace

r += dabs(x[i]-y[i])**p by r += dabs(dabs(x[i]-y[i])**p) And it does compile, but anyway I'm highly interested if you find a cleaner solution,

mgrover1 commented 1 year ago

I will give that a try - thanks so much @wolfidan !

mgrover1 commented 1 year ago

@wolfidan - we are still seeing failures in the continuous integration/testing - did you see similar errors?https://github.com/ARM-DOE/pyart/actions/runs/5599879973/jobs/10241541373

wolfidan commented 1 year ago

Hi @mgrover1 , in pyart/map/grid_mapper.py, there is a leafsize = 10. (with dot), you have to replace it by 10 (in int). I think

mgrover1 commented 1 year ago

Ahhhhhh good catch - the issue is the power operator is treated differently in 3.0 compared to previous versions...

https://cython.readthedocs.io/en/latest/src/userguide/migrating_to_cy30.html

Cython 3 has changed the behaviour of the power operator to be more like Python. The consequences are that

a**b of two ints may return a floating point type,

a**b of one or more non-complex floating point numbers may return a complex number.

The old behaviour can be restored by setting the cpow [compiler directive](https://cython.readthedocs.io/en/latest/src/userguide/source_files_and_compilation.html#compiler-directives) to True.

We can change this behavior by setting cpow=True in the setup.py configuration.

wolfidan commented 1 year ago

Wow great, thtat you found that!