Skip to content

DEBUG allow up to 65536 bins in HGBT #28638

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

DEBUG allow up to 65536 bins in HGBT #28638

wants to merge 12 commits into from

Conversation

lorentzenchr
Copy link
Member

@lorentzenchr lorentzenchr commented Mar 16, 2024 •
edited by ogrisel
Loading

Reference Issues/PRs

Debug PR for #28603.

For debugging https://dev.azure.com/scikit-learn/scikit-learn/_build/results?buildId=65063&view=logs&j=84d10a00-2692-50a3-2b29-4c80601878c4 on Windows pymin_conda_forge_mkl which fails in the Install step with

"C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\bin\HostX86\x64\link.exe" /nologo /INCREMENTAL:NO /LTCG /DLL /MANIFEST:EMBED,ID=2 /MANIFESTUAC:NO /LIBPATH:C:\Miniconda\envs\testvenv\libs /LIBPATH:C:\Miniconda\envs\testvenv /LIBPATH:C:\Miniconda\envs\testvenv\PCbuild\amd64 /LIBPATH:build\temp.win-amd64-cpython-39 "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\ATLMFC\lib\x64" "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\lib\x64" "/LIBPATH:C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\lib\um\x64" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\lib\10.0.22621.0\ucrt\x64" "/LIBPATH:C:\Program Files (x86)\Windows Kits\10\\lib\10.0.22621.0\\um\x64" libsvm-skl.lib liblinear-skl.lib /EXPORT:PyInit__vector_sentinel build\temp.win-amd64-cpython-39\Release\sklearn\utils\_vector_sentinel.obj /OUT:build\lib.win-amd64-cpython-39\sklearn\utils\_vector_sentinel.cp39-win_amd64.pyd /IMPLIB:build\temp.win-amd64-cpython-39\Release\sklearn\utils\_vector_sentinel.cp39-win_amd64.lib /openmp
LINK : warning LNK4044: unrecognized option '/openmp'; ignored
   Creating library build\temp.win-amd64-cpython-39\Release\sklearn\utils\_vector_sentinel.cp39-win_amd64.lib and object build\temp.win-amd64-cpython-39\Release\sklearn\utils\_vector_sentinel.cp39-win_amd64.exp
Generating code
Finished generating code
Finished generating code
error: command 'C:\\Program Files\\Microsoft Visual Studio\\2022\\Enterprise\\VC\\Tools\\MSVC\\14.38.33130\\bin\\HostX86\\x64\\cl.exe' failed with exit code 2

As CI is green until and including ENH add Bitsets extension type, also with Cython 3.0.9 alone. So the Windows error comes from ENH add BinnedData extension type for mixed uint8 uint16 data.

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 16, 2024 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: b56f69c. Link to the linter CI: here

@lorentzenchr lorentzenchr force-pushed the debug_hgbt_variable_bins branch from 1aed35d to 2d37782 Compare March 16, 2024 14:37
@lorentzenchr lorentzenchr force-pushed the debug_hgbt_variable_bins branch from 2d37782 to ccd8f92 Compare March 16, 2024 15:22
@lorentzenchr lorentzenchr force-pushed the debug_hgbt_variable_bins branch from 8ee9ec9 to 257c075 Compare March 17, 2024 13:00
@ogrisel ogrisel mentioned this pull request Mar 18, 2024
Merged
@ogrisel
Copy link
Member

ogrisel commented Mar 18, 2024

From the windows log of the first failing build (the one without /DEBUG), I see the following error C2039: '__pyx_mstate_global': is not a member of '__pyx_mstate' reports hidden among warnings:

"C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\bin\HostX86\x64\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -IC:\Miniconda\envs\testvenv\lib\site-packages\numpy\core\include -IC:\Miniconda\envs\testvenv\include -IC:\Miniconda\envs\testvenv\Include "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um" /Tcsklearn\ensemble\_hist_gradient_boosting\_binning.c /Fobuild\temp.win-amd64-cpython-39\Release\sklearn\ensemble\_hist_gradient_boosting\_binning.obj /O2 /openmp
_binning.c
Finished generating code
sklearn\ensemble\_hist_gradient_boosting\_binning.c(19185): warning C4244: '=': conversion from 'const __pyx_t_7sklearn_5utils_9_typedefs_uint16_t' to '__pyx_t_7sklearn_5utils_9_typedefs_uint8_t', possible loss of data
sklearn\ensemble\_hist_gradient_boosting\_binning.c(19208): warning C4244: '=': conversion from 'Py_ssize_t' to 'int', possible loss of data
sklearn\ensemble\_hist_gradient_boosting\_binning.c(19609): warning C4244: '=': conversion from 'Py_ssize_t' to 'int', possible loss of data
sklearn\ensemble\_hist_gradient_boosting\_binning.c(21513): error C2039: '__pyx_mstate_global': is not a member of '__pyx_mstate'
sklearn\ensemble\_hist_gradient_boosting\_binning.c(3346): note: see declaration of '__pyx_mstate'building 'sklearn.ensemble._hist_gradient_boosting._predictor' extension
"C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\bin\HostX86\x64\cl.exe" /c /nologo /O2 /W3 /GL /DNDEBUG /MD -DNPY_NO_DEPRECATED_API=NPY_1_7_API_VERSION -IC:\Miniconda\envs\testvenv\lib\site-packages\numpy\core\include -IC:\Miniconda\envs\testvenv\include -IC:\Miniconda\envs\testvenv\Include "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.38.33130\ATLMFC\include" "-IC:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\VS\include" "-IC:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt" "-IC:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt" "-IC:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um" /Tcsklearn\ensemble\_hist_gradient_boosting\_predictor.c /Fobuild\temp.win-amd64-cpython-39\Release\sklearn\ensemble\_hist_gradient_boosting\_predictor.obj /O2 /openmp

_predictor.c
sklearn\ensemble\_hist_gradient_boosting\_binning.c(21532): error C2039: '__pyx_mstate_global': is not a member of '__pyx_mstate'
sklearn\ensemble\_hist_gradient_boosting\_predictor.c(18887): warning C4244: 'function': conversion from '__pyx_t_7sklearn_5utils_9_typedefs_intp_t' to 'int', possible loss of datasklearn\ensemble\_hist_gradient_boosting\_binning.c(3346): note: see declaration of '__pyx_mstate'

Finished generating code

Maybe this is a case of cython/cython#6052 ?

@lorentzenchr lorentzenchr mentioned this pull request Mar 18, 2024
Closed
@lorentzenchr lorentzenchr mentioned this pull request Apr 1, 2024
Merged
@lorentzenchr lorentzenchr force-pushed the debug_hgbt_variable_bins branch from c69cd9c to b56f69c Compare April 1, 2024 20:40
@lorentzenchr
Copy link
Member Author

Finding: Cython 3.0.10 seems to fix the problematic windows MSVC build.

@lorentzenchr lorentzenchr deleted the debug_hgbt_variable_bins branch April 3, 2024 18:55
@lorentzenchr lorentzenchr mentioned this pull request Apr 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
cython module:ensemble module:utils No Changelog Needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@lorentzenchr @ogrisel