scikit-learn-intelex
scikit-learn-intelex copied to clipboard
intel
[bug] fix preview get_patch_map bug in sklearnex
Description
Any assert in test_monkeypatch will leave the patch in place. This is very much an undesired behavior. Using a try finally statement will guarantee the use of unpatch_sklearn in the case of a failure (thereby limiting the scope of the patch to the test). Secondarily, test_patch_by_list_many_estimators was missing an unpatch call, which means it was impacting all tests following it. Thirdly, there is an issue where the get_patch_map lru_cache was only checking _is_preview_enabled on first cache, as lru_cache looks at *args and **kwargs for similarity to previous inputs. unpatch_sklearn clears that cache, which fixed the issue previously in the test. A wrapper was added to require checking for preview when trying to recover the lru_cache. Finally test_parallel is modified so that it properly unloads patch_sklearn at the end of the test.
Changes proposed in this pull request:
- Introduce try finally for every sklearn_patch call in all tests of test_monkeypatch.py
- Add unpatch_sklearn to test_patch_by_list_many_estimators
- Reintroduce test_monkeypatch into standard CI run call
- wrap get_patch_map to properly check for preview
- modify test_parallel.py to also respect global scope for patch_sklearn
private CI rerun to see if linear regression fail on GPU is related http://intel-ci.intel.com/eec0d542-000a-f1a2-b8f0-a4bf010d0e2e
CI rerun due to private CI issue: http://intel-ci.intel.com/eec9ad1f-d7de-f13f-a11c-a4bf010d0e2e
Turns out that whoever programmed the patch_map was abusing lru_cache to make get_patch_map into a state machine, rather than setting a module attribute or making a class they went with this. I can't dump the lru_cache without doing a much larger refactor, but I think I can devise a way to at least make it not be buggy while also allowing for preview. This absolutely needs a set of tickets for a rewrite, and is a ticking timebomb for anyone who does anything other than just vanilla patch_sklearn and unpatch_sklearn when combined with preview. The original daal4py author should have at least wrote a tiny note to their machinations, but then again that daal4py/sklearn/monkeypatch code is generally abhorrent.