Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade to python 3.12 #2292

Merged
merged 7 commits into from
Aug 1, 2024
Merged

Upgrade to python 3.12 #2292

merged 7 commits into from
Aug 1, 2024

Conversation

MikeSullivan7
Copy link
Collaborator

Issue

Closes #2267
Requires #2280 and #2291

Description

This PR upgrades MI to be built with Python 3.12, the latest version of Python.
This PR will need to be rebased after #2280 is merged!

Testing

make check
make test-system

Acceptance Criteria

Run all tests and check that MI runs as normal without any issues or warnings, this transition should be seemless.

Documentation

Release note

@MikeSullivan7 MikeSullivan7 added Type: Improvement Component: Core dependencies Pull requests that update a dependency file labels Jul 23, 2024
@MikeSullivan7 MikeSullivan7 self-assigned this Jul 23, 2024
@MikeSullivan7 MikeSullivan7 force-pushed the 2267_python_3.12_upgrade branch 4 times, most recently from 012200b to c5d44b5 Compare July 25, 2024 09:32
@coveralls
Copy link

coveralls commented Jul 25, 2024

Coverage Status

coverage: 74.316% (+1.3%) from 73.051%
when pulling 3b21da6 on 2267_python_3.12_upgrade
into 4370485 on main.

@MikeSullivan7
Copy link
Collaborator Author

In the Github Actions we get the following error

Run python -m pytest --cov --cov-report=xml -n auto -o log_cli=true --ignore=mantidimaging/eyes_tests --durations=10
##[debug]C:\Program Files\Git\bin\bash.EXE -l D:\a\_temp\d44cfc21-81ce-4ac6-89d0-faf82ec53d38.sh
C:\Users\runneradmin\miniconda3\envs\mantidimaging-dev\Lib\site-packages\_pytest\config\__init__.py:331: PluggyTeardownRaisedWarning: A plugin raised an exception during an old-style hookwrapper teardown.
Plugin: helpconfig, Hook: pytest_cmdline_parse
ConftestImportFailure: ImportError: DLL load failed while importing astra_c: The specified module could not be found. (from D:\a\mantidimaging\mantidimaging\conftest.py)
For more information see https://pluggy.readthedocs.io/en/stable/api_reference.html#pluggy.PluggyTeardownRaisedWarning
  config = pluginmanager.hook.pytest_cmdline_parse(
ImportError while loading conftest 'D:\a\mantidimaging\mantidimaging\conftest.py'.
conftest.py:11: in <module>
    from mantidimaging.core.utility.leak_tracker import leak_tracker
mantidimaging\core\__init__.py:5: in <module>
    from . import operations, io, parallel, utility  # noqa: F401
mantidimaging\core\io\__init__.py:5: in <module>
    from . import (  # noqa: F401
mantidimaging\core\io\saver.py:[20](https://github.com/mantidproject/mantidimaging/actions/runs/10096299143/job/27918470051#step:12:21): in <module>
    from ..operations.rescale import RescaleFilter
mantidimaging\core\operations\rescale\__init__.py:5: in <module>
    from .rescale import RescaleFilter  # noqa:F401
mantidimaging\core\operations\rescale\rescale.py:11: in <module>
    from mantidimaging.gui.utility.qt_helpers import Type
mantidimaging\gui\__init__.py:5: in <module>
    from .gui import execute  # noqa: F401  # noqa:F8[21](https://github.com/mantidproject/mantidimaging/actions/runs/10096299143/job/27918470051#step:12:22)
mantidimaging\gui\gui.py:13: in <module>
    from mantidimaging.gui.windows.main import MainWindowView
mantidimaging\gui\windows\main\__init__.py:6: in <module>
    from .view import MainWindowView  # noqa: F401
mantidimaging\gui\windows\main\view.py:39: in <module>
    from mantidimaging.gui.windows.recon import ReconstructWindowView
mantidimaging\gui\windows\recon\__init__.py:5: in <module>
    from .model import ReconstructWindowModel  # noqa: F401
mantidimaging\gui\windows\recon\model.py:12: in <module>
    from mantidimaging.core.reconstruct import get_reconstructor_for
mantidimaging\core\reconstruct\__init__.py:7: in <module>
    from .astra_recon import AstraRecon
mantidimaging\core\reconstruct\astra_recon.py:10: in <module>
    import astra
C:\Users\runneradmin\miniconda3\envs\mantidimaging-dev\Lib\site-packages\astra\__init__.py:[26](https://github.com/mantidproject/mantidimaging/actions/runs/10096299143/job/27918470051#step:12:27): in <module>
    from . import matlab as m
C:\Users\runneradmin\miniconda3\envs\mantidimaging-dev\Lib\site-packages\astra\matlab.py:38: in <module>
    from . import astra_c
E   ImportError: DLL load failed while importing astra_c: The specified module could not be found.

However when looking in C:/Users/runneradmin/miniconda3/envs/mantidimaging-dev/Lib/site-packages/astra we have the following files:

__init__.py
__pycache__
algorithm.py
algorithm_c.cp311-win_amd64.pyd
astra.py
astra_c.cp311-win_amd64.pyd
creators.py
data2d.py
data2d_c.cp311-win_amd64.pyd
data3d.py
data3d_c.cp311-win_amd64.pyd
experimental.cp311-win_amd64.pyd
extrautils.cp311-win_amd64.pyd
functions.py
log.py
log_c.cp311-win_amd64.pyd
matlab.py
matrix.py
matrix_c.cp311-win_amd64.pyd
optomo.py
plugin.py
plugin_c.cp311-win_amd64.pyd
plugins
projector.py
projector_c.cp311-win_amd64.pyd
projector3d.py
projector3d_c.cp311-win_amd64.pyd
pythonutils.py
tests.py
utils.cp311-win_amd64.pyd

Where we can see the file astra_c.cp311-win_amd64.pyd exists, so it is confusing as to why we get the import error.....

@MikeSullivan7 MikeSullivan7 force-pushed the 2267_python_3.12_upgrade branch from 04250fa to c5d44b5 Compare July 25, 2024 15:11
@MikeSullivan7 MikeSullivan7 added the rebuild_docker 🐋 Add if you want to force rebuild docker images (ONLY IF MERGING INTO MAIN) label Jul 25, 2024
@samtygier-stfc
Copy link
Collaborator

Does it make any difference if you remove astra-toolbox from the channels in the environment files?

@MikeSullivan7
Copy link
Collaborator Author

Does it make any difference if you remove astra-toolbox from the channels in the environment files?

I tried this locally in environment-dev.yaml and it didn't seem to have much of an effect, but will try again to make sure as I may have changed other things since then

@MikeSullivan7
Copy link
Collaborator Author

So after running pytest on a fresh install of Windows 10 on a virtual machine, I get the same error as the Github actions:
image

Could potentially be Cuda related then

@MikeSullivan7
Copy link
Collaborator Author

So in version Astra 2.0.0 there is an astra_c.py file which is not in version 2.1.0, which may be causing the problems

@MikeSullivan7
Copy link
Collaborator Author

MikeSullivan7 commented Jul 30, 2024

I noticed that between the package builds on my machine and a fresh install, some of the astra dependencies are different builds, i.e. for libastra we have build cuda11.2hb8489e9_0 on a fresh install vs cuda11.8hb8fb34d_111 on my local machine.

When attempting to match the builds on the virtual machine, I get this:
image

Therefore it seems that it is not possible to run this version of Astra unless you have a full CUDA install and therefore need a GPU.

NOTE: This is also the case for python 3.10

@MikeSullivan7
Copy link
Collaborator Author

Ive set the env variable at the stage just before building the dev environment and it seems to do the job!

@MikeSullivan7 MikeSullivan7 force-pushed the 2267_python_3.12_upgrade branch from ddfb8aa to d6e2bcb Compare July 31, 2024 08:02
@samtygier-stfc
Copy link
Collaborator

Looks good from some quick local testing, but lets wait until the pyinstaller thing is understood.

@MikeSullivan7 MikeSullivan7 force-pushed the 2267_python_3.12_upgrade branch from 902e0a8 to f3f8fbc Compare August 1, 2024 13:31
@MikeSullivan7
Copy link
Collaborator Author

PyInstaller now builds without CUDA errors, worked out that some files were being missed in cupy_backends after the cupy upgrade, so these have now been added explicitly as a submodule in PackagingWithPyInstaller.py :)

@MikeSullivan7 MikeSullivan7 force-pushed the 2267_python_3.12_upgrade branch from f3f8fbc to 3b21da6 Compare August 1, 2024 15:33
Copy link
Collaborator

@samtygier-stfc samtygier-stfc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I've run system and unit tests locally. Run operations and reconstruction loclaly. Built a pyinstaller and tested it.

@samtygier-stfc samtygier-stfc added this pull request to the merge queue Aug 1, 2024
Merged via the queue into main with commit 64dfcee Aug 1, 2024
6 checks passed
@samtygier-stfc samtygier-stfc deleted the 2267_python_3.12_upgrade branch August 1, 2024 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core dependencies Pull requests that update a dependency file rebuild_docker 🐋 Add if you want to force rebuild docker images (ONLY IF MERGING INTO MAIN) Type: Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Python to 3.12
3 participants