BUG: MAC version preprocessor shim #11
Conversation
* tries to address this: scipy/scipy#20300 * I needed the preceding underscores added here to produce `__MAC_OS_X_VERSION_MIN_REQUIRED` and actually populate a value into this preprocessor "variable" * After that adjustment locally, I see this from carefully crafted pragma messages: `__MAC_OS_X_VERSION_MIN_REQUIRED: 140000` and `MAC_OS_X_VERSION_10_15: 101500 ` (the latter "variable" was already populated) * of course, I'm on a way newer MacOS (14.4) than the problem this patch attempts to deal with, but to be fair I was reproducing the problem of `MAC_OS_VERSION_MIN_REQUIRED` not being populated with a constant value nonetheless
|
I suppose there isn't CI on this repo... given that SciPy CI was passing before this change, I recommend we just try it and see if it fixes for the original issue authors |
|
Yes, CI is missing, since this repo has ben mostly dormant for several years. I'll see if I can add something (at least for compilation), but the trick will be to cover all the unusual OSs and OS versions that cause trouble. That said, perhaps we should simply give up on the idea of a portable |
|
Is this something that needs to be settled very urgently, or do we have time to try a version that unconditionally uses the home-grown aligned allocation? |
It needs to be settled urgently for NumPy and SciPy, but we could apply the patch in the fork we maintain under the SciPy organisation, if you would like to try a more substantial fix in this repo. EDIT: numpy/numpy#25940 also reported an issue on Termux |
|
The (somewhat) more substantial patch I have in mind would be to simply remove the But if you prefer to apply the proposed one-liner for the next |
This may be preferable, as I don't know whether anyone have found a patch which works for the Termux issue.
This also sounds sensible, maybe no harm in merging this now then considering other options. |
|
Great, let's do it like this! |
|
Done! You can now choose between |
* related to scipygh-20300, though awaits `conda-forge` testing on old MacOS versions for actual confirmation of fix * pulls in changes described at mreineck/pocketfft#11 and scipy/pocketfft#2 * I'm honestly not sure how much we gain by being conservative and not just turning the code path off entirely (mreineck/pocketfft@33ae5dc); perhaps there are some performance advantages or something, but if we have any more trouble with this we should obviously just do that (and I suspect that's what NumPy should do for numpy/numpy#25940) [skip cirrus] [skip circle]
* related to scipygh-20300, though awaits `conda-forge` testing on old MacOS versions for actual confirmation of fix * pulls in changes described at mreineck/pocketfft#11 and scipy/pocketfft#2 * I'm honestly not sure how much we gain by being conservative and not just turning the code path off entirely (mreineck/pocketfft@33ae5dc); perhaps there are some performance advantages or something, but if we have any more trouble with this we should obviously just do that (and I suspect that's what NumPy should do for numpy/numpy#25940) [skip cirrus] [skip circle]
* related to scipygh-20300, though awaits `conda-forge` testing on old MacOS versions for actual confirmation of fix * pulls in changes described at mreineck/pocketfft#11 and scipy/pocketfft#2 * I'm honestly not sure how much we gain by being conservative and not just turning the code path off entirely (mreineck/pocketfft@33ae5dc); perhaps there are some performance advantages or something, but if we have any more trouble with this we should obviously just do that (and I suspect that's what NumPy should do for numpy/numpy#25940) [skip cirrus] [skip circle]
"Upstream" version of scipy#1 (@lucascolley suggested we at least flush the CI, regardless of where we merge it, if we merge it..)
tries to address this: BUG: SciPy 1.13.0 not buildable on old macOS due to pocketfft code scipy/scipy#20300
I needed the preceding underscores added here to produce
__MAC_OS_X_VERSION_MIN_REQUIREDand actually populate a value into this preprocessor "variable"After that adjustment locally, I see this from carefully crafted pragma messages:
__MAC_OS_X_VERSION_MIN_REQUIRED: 140000andMAC_OS_X_VERSION_10_15: 101500(the latter "variable" was already populated)of course, I'm on a way newer MacOS (14.4) than the problem this patch attempts to deal with, but to be fair I was reproducing the problem of
MAC_OS_VERSION_MIN_REQUIREDnot being populated with a constant value nonetheless