-
Notifications
You must be signed in to change notification settings - Fork 879
GPU optimizations #1812
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
base: main
Are you sure you want to change the base?
GPU optimizations #1812
Conversation
omri374
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! A long due proper support for GPU workloads. Left some comments to consider.
presidio-analyzer/presidio_analyzer/nlp_engine/device_detector.py
Outdated
Show resolved
Hide resolved
benchmark_presidio.py
Outdated
| @@ -0,0 +1,606 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider putting the benchmark result files in a dedicated folder under docs, or omit them from the repo. There's a chance for this to become stale very quickly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its only for the GPU tests. i think we will need something more organized later on.
presidio-analyzer/presidio_analyzer/nlp_engine/stanza_nlp_engine.py
Outdated
Show resolved
Hide resolved
|
|
||
| logger.debug(f"Loading SpaCy and transformers models: {self.models}") | ||
|
|
||
| # Configure GPU if available |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Already called in the super
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed.
presidio-analyzer/pyproject.toml
Outdated
| "phonenumbers (>=8.12,<10.0.0)", | ||
| "pydantic (>=2.0.0,<3.0.0)" | ||
| "pydantic (>=2.0.0,<3.0.0)", | ||
| "cupy-cuda12x>=13.4.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this install cuda? Will the work for CPU only machines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but I believe it should be in the new GPU section to give users more flexibility regarding GPU dependencies. and not install what not in use.
This PR is not ready for full review yet. I mainly want someone with a different GPU to run it and check whether the GPU-pref optimization improves performance on their setup. Before finalizing, I’ll remove the benchmark script—I kept it only so others can measure GPU improvements on their machines. I’m also considering adding an extra gpu section in the pyproject.toml, so people can either install a common set of GPU dependencies or use their own GPU libraries. |
Coverage report (presidio-analyzer)Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@RonShakutai Is this meant to be support inside the docker image without additional changes? |
Yes, the device detection is automatic and requires no code changes |
Doesn't GPU require drivers on the OS level? Not sure we install those currently. |
GPU execution still requires OS-level drivers (CUDA, NVIDIA runtime), which are outside the scope of this PR. This PR focuses on two things: Correct GPU usage in code paths for Stanza, spaCy, GLiNER, and Transformers, once a GPU is available. Providing an optional gpu extra that installs commonly used CUDA-compatible Python dependencies, so users can “plug and go” in most setups. CUDA versions and drivers are highly GPU-specific and must be installed by the GPU owner, who knows their hardware best. For that reason, we do not bundle or enforce CUDA drivers. We only add a recommended GPU dependency set via an extra in pyproject.toml. |
For this to work in Docker, we need two things: (1) code level adjusments, as done in this PR, (2) Dockerfile adjustments, as our current Dockerfile doesn't install cuda/cudnn packages (e.g. |
…S initialization failures
…crosoft/presidio into ronshakutai/gpu-optimizations align to main
Totally agree. @omri374 For Docker concern that was raised correctly by @tamirkamara !, this is a separate concern. |
Change Description
This PR introduce GPU optimizations for Gliner, Spacy, stanza and transformers.
Technical Implementation
DeviceDetectorsingleton for automatic GPU detection and CUDA initializationcupy-cuda12xThis PR introduces GPU handling improvements for GLiNER, spaCy, Transformers, and Stanza NLP engines, optimizing GPU detection and utilization.
Reproduce Results
On this branch:
cd presidio-analyzer poetry run python ../benchmark_presidio.py --engines spacy/transformers/gliner/stanza --sizes 50,500 --json gpu_results.jsonThen download the script, switch to main branch, and re-run: pay attention to change the json results file name in the command.
Compare
gpu_results.jsonvsmain_results.json.Results!
GLiNER - Big Improvement
Transformers - Big Improvement
Comparison of Transformers (StanfordAIMI/stanford-deidentifier-base)
Stanza - Big Improvement
spaCy - No Change
Comparison of spaCy (en_core_web_lg) performance before and after GPU handling improvements.
Checklist