FLAMEGPU / FLAMEGPU2

FLAME GPU 2 is a GPU accelerated agent based modelling framework for CUDA C++ and Python
https://flamegpu.com
MIT License
99 stars 19 forks source link

Python 3.12 and 3.7 #1117

Closed ptheywood closed 9 months ago

ptheywood commented 9 months ago
ptheywood commented 9 months ago

I may have been a little keen for this (python 3.12 released today, 2023-10-02), might need to wait a few days and re-run the CI.

(Edit: the actions/setup-python checks for new python at 3am and 3pm each day, opening a PR if one is avialable and worked. Todays failed due to download errors. I'd expect this won't take too long to resolve though)

The latest the action can currently deal with is 3.12-rc3.

The manylinux container(s) however are already compatible, so that should indicate if there are any game-breaking changes in 3.12 / swig incompatibility (I hope not)

ptheywood commented 9 months ago

Having said that, on looking at the python3.12 release notes, there are deprecateionWArnings added for parts of ast which are used, i.e. ast.Num which is used for a python 3.7 thing, so maybe we will want/need to remove that, or guard it by python version instead?

ptheywood commented 9 months ago

As expected when running the generated wheels with python 3.12 Deprecation warnings related to ast components are emitted

python/codegen/test_codegen.py: 57 warnings
python/codegen/test_codegen_integration.py: 2 warnings
  /home/ptheywood/miniconda3/envs/py312/lib/python3.12/site-packages/pyflamegpu/codegen/codegen.py:588: DeprecationWarning: ast.Str is deprecated and will be removed in Python 3.14; use ast.Constant instead
    elif isinstance(tree.value, ast.Str):

python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
  /home/ptheywood/miniconda3/envs/py312/lib/python3.12/site-packages/pyflamegpu/codegen/codegen.py:226: DeprecationWarning: ast.Num is deprecated and will be removed in Python 3.14; use ast.Constant instead
    if isinstance(i, ast.Num): # num required for python 3.7

python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
  /home/ptheywood/miniconda3/envs/py312/lib/python3.12/site-packages/pyflamegpu/codegen/codegen.py:227: DeprecationWarning: Attribute n is deprecated and will be removed in Python 3.14; use value instead
    if not isinstance(i.n, int):

python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
python/codegen/test_codegen.py::CodeGenTest::test_fgpu_macro_environment
  /home/ptheywood/miniconda3/envs/py312/lib/python3.12/site-packages/pyflamegpu/codegen/codegen.py:229: DeprecationWarning: Attribute n is deprecated and will be removed in Python 3.14; use value instead
    cpp_func_name += f", {i.n}"

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

These should probably be addressed prior to merge.

Otherwise all tests passed.

650 passed, 11 skipped, 69 warnings in 3080.89s (0:51:20)

Although they did take a very long time, however re-running is significalnlty faster, so ther seems to be some nvrtc perf regressions from newer CUDAs rather than python changes

650 passed, 11 skipped, 69 warnings in 3.19s 
ptheywood commented 9 months ago

I've now prevented python 3.12 (actually 3.8+) from calling the deprecated methods which are included in codegen.py for python 3.7 support, by comparing the version tuple (sys.version_info).

Tested with python 3.7 and 3.12, demonstrating that both pass and 3.12 doesn't generated deprecation warnings.

$ python3 -m pytest ../tests/python/codegen/test_codegen.py 
================================= test session starts =================================
platform linux -- Python 3.12.0, pytest-7.4.2, pluggy-1.3.0
rootdir: /home/ptheywood/code/flamegpu/FLAMEGPU2
collected 51 items                                                                    

../tests/python/codegen/test_codegen.py ....................................... [ 76%]
............                                                                    [100%]

================================= 51 passed in 0.15s ==================================
$ python3 -m pytest ../tests/python/codegen/test_codegen.py
========================================= test session starts ==========================================
platform linux -- Python 3.7.0, pytest-7.4.2, pluggy-1.2.0
rootdir: /home/ptheywood/code/flamegpu/FLAMEGPU2
collected 51 items                                                                                     

../tests/python/codegen/test_codegen.py ...................................................      [100%]

========================================== 51 passed in 0.14s ==========================================

Edit:

Full test suites passed with python 3.7 and 3.12 too (using CUDA 12.1 to avoid the 12.2 RTC perf issues)

I've now also tested with python 3.8 as well which passed tests with no warnings.

Robadob commented 9 months ago

I'll let @mondus review this, I barely understand how it was setup. So I'm never sure what the safe way of handling stuff is (e.g. if that's deprecated, do we need to handle it a different way?)