fix: fall back to onnxruntime/coremltools on Python 3.12+ - #204
Open
bodapatisaikrishna wants to merge 1 commit into
Open
bodapatisaikrishna wants to merge 1 commit into
bodapatisaikrishna wants to merge 1 commit into
Conversation
tensorflow/tensorflow-macos never published a cp312 wheel below version 2.16 (verified against PyPI release metadata), but the dependency markers cap both at <2.15.1 while still activating for python_version >= 3.11 (non-Darwin) / > 3.11 (Darwin) -- an unsatisfiable constraint on Python 3.12. Fixes spotify#203 (same root cause as spotify#159, spotify#188). Simply raising the version cap is not safe: tensorflow-macos==2.16.2 fails to load the bundled SavedModel with AttributeError("'_UserObject' object has no attribute 'add_slot'"), Keras 3's optimizer object graph being incompatible with the old checkpoint. Verified this empirically before choosing the fix below. basic_pitch already bundles the model in ONNX/TFLite/CoreML formats and already has fallback backend-selection logic. On Python 3.12+: exclude tensorflow(-macos) (both in `dependencies` and the `[tf]` extra) so it isn't attempted where it can't work; add onnxruntime (has cp312+ wheels) for non-Darwin, since coremltools is already an unconditional Darwin dependency and becomes the default there for free. Added the Python :: 3.12 classifier. Verified ONNX and CoreML backends produce output matching the current TensorFlow 2.15.0 path (28 note events, matching sums) via direct predict() calls, and verified the full fix end-to-end on real Python 3.12.0 macOS arm64: install resolves cleanly, auto-selects coremltools, and produces matching inference output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #203 (and the same root cause behind #159 and #188): on Python 3.12,
pip install basic-pitchfails to resolve at all, on any platform.tensorflow/tensorflow-macosnever published acp312wheel below version2.16 (verified against PyPI's release metadata), but the dependency markers
cap both at
<2.15.1while still activating forpython_version >= '3.11'(non-Darwin) /
python_version > '3.11'(Darwin) -- an unsatisfiableconstraint on 3.12.
Simply raising the version cap is not a safe fix. I installed
tensorflow-macos==2.16.2and confirmed the bundledsaved_models/icassp_2022/nmpSavedModel fails to load:
This is Keras 3 (TF 2.16's default) being unable to reconstruct the old
optimizer's object graph inside the SavedModel checkpoint. So relaxing the pin
would trade an install-time failure for a worse runtime one.
Fix
basic_pitchalready bundles the model in three other formats(
nmp.onnx,nmp.tflite,nmp.mlpackage) and already has fallback logic inbasic_pitch/__init__.py(TF_PRESENT→CT_PRESENT→TFLITE_PRESENT→ONNX_PRESENT) to pick whichever backend is installed. On Python 3.12+:coremltoolsis already an unconditional dependency there, so nonew dependency is needed -- it becomes the default backend automatically.
onnxruntime(hascp312+ wheels) as thedependency, so
onnxruntimebecomes the default backend.tensorflow/tensorflow-macos(both independenciesand the[tf]extra)are now excluded on
python_version >= '3.12'instead of installing somethingthat can't load the model. Added the
Python :: 3.12classifier.Testing Instructions
Verified with real inference, not just
pip install --dry-run:Confirmed the vulnerability/bug is real:
tensorflow-macos==2.16.2raises the
add_sloterror above when loading the bundled model.Confirmed ONNX and CoreML produce matching output to the current
TensorFlow 2.15.0 path, by running
basic_pitch.inference.predict()ontests/resources/vocadito_10.wavagainst all three backends directly:nmp.onnx)nmp.mlpackage)Confirmed the fix end-to-end on real Python 3.12.0, macOS arm64 (the
exact environment from macOS Python 3.12 dependency marker selects TensorFlow-macOS without cp312 wheels #203's repro):
uv pip install --dry-runnowresolves cleanly (installs
coremltools, notensorflow-macosattempted);a real install correctly reports
TF_PRESENT: False, CT_PRESENT: True,auto-selects the
.mlpackagemodel, andpredict()produces the same 28note events with matching output sums as the TensorFlow 2.15.0 baseline.
Ran
tests/test_inference.pyon both the existing Python 3.11 (TF) pathand the new Python 3.12 (CoreML) path: 8/9 pass on 3.12. The one failure
(
test_predict) is a pre-existing, unrelated issue --librosa.get_duration(filename=...)was deprecated in librosa 0.10.0 and removed in 1.0.0, which is what a
fresh
librosa>=0.8.0install now resolves to. I did not fix it heresince
path=(the replacement kwarg) doesn't exist before librosa 0.10.0,and
basic-pitchsupportslibrosa>=0.8.0with no upper bound -- aversion-safe fix is a separate, unrelated piece of work. Verified this
failure is unrelated to this PR by reproducing it on the current Python
3.11/TensorFlow path too (same error, same cause, present on
main).Additional Information