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

Autogenerate environment.yml file from pyproject.toml #12914

Merged
merged 26 commits into from
Oct 25, 2024

Conversation

drammock
Copy link
Member

closes #12903

Easiest to review a few specific commits:

  • a3f2a0e adds the hook
  • 1a0d7b9 does nothing except sort environment.yml entries so that subsequent hook-induced changes to that file are easier to see
  • ef61f28 shows the actual changes caused by the hook on first run (plus a fake "aaaaaaa" dep added to trigger the hook, which is removed in subsequent commit 4e99502)

Note that this does not address #12903 (comment) --- I figured easier to get this one in / tested and then deal later with cross-repo syncing.

.conda-extra-dependencies.yml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
tools/hooks/update_environment_file.py Outdated Show resolved Hide resolved
@drammock
Copy link
Member Author

windows-latest mamba job will continue to fail until the fix for mamba-org/mamba#3467 has been released.

environment.yml Outdated Show resolved Hide resolved
Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

It occurred to me that any autogenerated file should have a comment so I just pushed a tiny commit to add this to the top of the env file:

# THIS FILE IS AUTO-GENERATED BY tools/hooks/update_environment_file.py AND WILL BE OVERWRITTEN

similar to what we have in doc/sphinxext/credit_tools.py

@larsoner larsoner enabled auto-merge (squash) October 24, 2024 17:19
@larsoner
Copy link
Member

Ughhhh looks like I was wrong about openblas, will push a commit

@larsoner
Copy link
Member

I am a bit mystified by both the pip-pre failure and the conda one. Looking at a diff of the sorted main YML and the one in this PR I don't see anything that should break linalg results as seems to be happening in the conda install now.

@drammock
Copy link
Member Author

I am a bit mystified by both the pip-pre failure and the conda one. Looking at a diff of the sorted main YML and the one in this PR I don't see anything that should break linalg results as seems to be happening in the conda install now.

I'm scratching my head too 😕

@drammock
Copy link
Member Author

the numpy config on windows-latest mamba is:

 Build Dependencies:
  blas:
    detection method: pkgconfig
    found: true
    include directory: C:/Users/runneradmin/micromamba/envs/mne/Library/include
    lib directory: C:/Users/runneradmin/micromamba/envs/mne/Library/lib
    name: blas
    openblas configuration: unknown
    pc file directory: D:\bld\numpy_1707225570061\_h_env\Library\lib\pkgconfig
    version: 3.9.0
  lapack:
    detection method: internal
    found: true
    include directory: unknown
    lib directory: unknown
    name: dep2598594164480
    openblas configuration: unknown
    pc file directory: unknown
    version: 1.26.4

The line blas: openblas configuration: unknown is different on my local machine, which has USE_64BITINT=1 DYNAMIC_ARCH=1 DYNAMIC_OLDER= NO_CBLAS= NO_LAPACK= NO_LAPACKE= NO_AFFINITY=1 USE_OPENMP= HASWELL MAX_THREADS=2. The version string also looks fishy (I see 0.3.23.dev locally, CI has 3.9.0?? AFAIK openblas versions are all still 0.x)

@drammock
Copy link
Member Author

comparing that to the windows pip-pre job:

Build Dependencies:
  blas:
    detection method: pkgconfig
    found: true
    include directory: C:/Users/runneradmin/AppData/Local/Temp/cibw-run-rk8_4lij/cp312-win_amd64/build/venv/Lib/site-packages/scipy_openblas64/include
    lib directory: C:/Users/runneradmin/AppData/Local/Temp/cibw-run-rk8_4lij/cp312-win_amd64/build/venv/Lib/site-packages/scipy_openblas64/lib
    name: scipy-openblas
    openblas configuration: OpenBLAS 0.3.28  USE64BITINT DYNAMIC_ARCH NO_AFFINITY
      Haswell MAX_THREADS=24
    pc file directory: D:/a/numpy/numpy/.openblas
    version: 0.3.28
  lapack:
    detection method: pkgconfig
    found: true
    include directory: C:/Users/runneradmin/AppData/Local/Temp/cibw-run-rk8_4lij/cp312-win_amd64/build/venv/Lib/site-packages/scipy_openblas64/include
    lib directory: C:/Users/runneradmin/AppData/Local/Temp/cibw-run-rk8_4lij/cp312-win_amd64/build/venv/Lib/site-packages/scipy_openblas64/lib
    name: scipy-openblas
    openblas configuration: OpenBLAS 0.3.28  USE64BITINT DYNAMIC_ARCH NO_AFFINITY
      Haswell MAX_THREADS=24
    pc file directory: D:/a/numpy/numpy/.openblas
    version: 0.3.28

here we see a reasonable version number, known configuration strings, and a different name ("scipy-openblas" instead of "blas")

@drammock
Copy link
Member Author

ok, yep: windows-latest mamba is getting an MKL version of numpy for some reason:

 + numpy                1.26.4 (MKL 2024.2.2-Product with 1 thread)

@drammock
Copy link
Member Author

disabling auto-merge because I'm not certain if 69a790d will actually be necessary anymore.

@@ -9,7 +9,7 @@ if [ "${TEST_MODE}" == "pip" ]; then
python -m pip install $STD_ARGS --only-binary="numba,llvmlite,numpy,scipy,vtk,dipy" -e .[test,full] "numpy<2"
elif [ "${TEST_MODE}" == "pip-pre" ]; then
${SCRIPT_DIR}/install_pre_requirements.sh
python -m pip install $STD_ARGS --pre -e .[test]
python -m pip install $STD_ARGS --pre -e .[test_extra]
Copy link
Member Author

Choose a reason for hiding this comment

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

the corresponding branch in github_actions_dependencies.sh uses test_extra too, and it's needed for utils/tests/test_config.py (I think previously it was passing on windows by chance)

@larsoner larsoner merged commit 0755ef0 into mne-tools:main Oct 25, 2024
28 checks passed
@larsoner
Copy link
Member

Thanks @drammock !

@drammock drammock deleted the autogen-env-yaml branch October 25, 2024 14:29
@drammock
Copy link
Member Author

@larsoner do you have any idea why we were suddenly getting MKL? I never did track down a cause.

@larsoner
Copy link
Member

No idea. Could be unfortunate timing with some conda change. Or something in the solver changing some logic. Or a new package. But I like the nomkl solution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: auto-generate environment.yaml from pyproject.toml
2 participants