Skip to content
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

[HuggingFace][Neuronx] Inference - Optimum Neuron 0.0.25 - Neuron sdk 2.20.0 - Transformers to 4.43.2 #4308

Merged
merged 14 commits into from
Oct 21, 2024

Conversation

dacorvo
Copy link
Contributor

@dacorvo dacorvo commented Oct 1, 2024

GitHub Issue #4307

Description

This PR creates Hugginface's PyTorch DLC for inference on neuron-v2 devices (namely Inferentia 2 and Trainium).

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@aws-deep-learning-containers-ci aws-deep-learning-containers-ci bot added build Reflects file change in build folder huggingface Reflects file change in huggingface folder sagemaker_tests sanity Reflects file change in dlc_tests/sanity folder Size:S Determines the size of the PR test Reflects file change in test folder labels Oct 1, 2024
@dacorvo dacorvo changed the title Add neuronx inf 0.0.25 [HuggingFace][Neuronx] Inference - Optimum Neuron 0.0.25 - Neuron sdk 2.20.0 - Transformers to 4.43.2 Oct 1, 2024
@dacorvo dacorvo force-pushed the add-neuronx-inf-0.0.25 branch from 889bc21 to cab4581 Compare October 2, 2024 07:23
@dacorvo dacorvo marked this pull request as ready for review October 2, 2024 07:24
@dacorvo dacorvo requested review from a team as code owners October 2, 2024 07:24
@dacorvo dacorvo force-pushed the add-neuronx-inf-0.0.25 branch 2 times, most recently from a5b116d to a373d48 Compare October 4, 2024 11:15
Copy link
Contributor

@Captainia Captainia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the changes, could you follow the PR template to trigger tests for PR hooks? You can edit the developer toml file based on instruction

# Set to "huggingface", for example, if you are a huggingface developer. Default is ""
.

@dacorvo dacorvo force-pushed the add-neuronx-inf-0.0.25 branch from dbc0f9e to 06824de Compare October 7, 2024 07:31
@dacorvo
Copy link
Contributor Author

dacorvo commented Oct 7, 2024

Thanks for making the changes, could you follow the PR template to trigger tests for PR hooks? You can edit the developer toml file based on instruction

I added a revertme commit with the toml changes.

@Captainia
Copy link
Contributor

Thanks for making the changes, could you follow the PR template to trigger tests for PR hooks? You can edit the developer toml file based on instruction

I added a revertme commit with the toml changes.

Checking with team why only the format check is triggered

@Captainia
Copy link
Contributor

/rerun

2 similar comments
@sirutBuasai
Copy link
Contributor

/rerun

@Captainia
Copy link
Contributor

/rerun

Copy link
Contributor

@Captainia Captainia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code build failures when building the docker file-

E: Unable to locate package libml2
--
407 |  
408 | The command '/bin/sh -c apt-get update  && apt-get upgrade -y  && apt-get install -y --no-install-recommends software-properties-common  && add-apt-repository ppa:openjdk-r/ppa  && apt-get update  && apt-get install -y --no-install-recommends     build-essential     apt-transport-https     ca-certificates     cmake     curl     emacs     git     jq     libgl1-mesa-glx     libsm6     libxext6     libxrender-dev     libgssapi-krb5-2     openjdk-11-jdk     vim     wget     unzip     zlib1g-dev     libcap-dev     gpg-agent     libexpat1     libml2  && rm -rf /var/lib/apt/lists/*  && rm -rf /tmp/tmp*  && apt-get clean' returned a non-zero code: 100
409 | [ERROR] ERROR during Docker BUILD
410 | [ERROR] Error message received for docker/2.1/py3/sdk2.20.0/Dockerfile.neuronx while docker build: {'errorDetail': {'code': 100, 'message': "The command '/bin/sh -c apt-get update  && apt-get upgrade -y  && apt-get install -y --no-install-recommends software-properties-common  && add-apt-repository ppa:openjdk-r/ppa  && apt-get update  && apt-get install -y --no-install-recommends     build-essential     apt-transport-https     ca-certificates     cmake     curl     emacs     git     jq     libgl1-mesa-glx     libsm6     libxext6     libxrender-dev     libgssapi-krb5-2     openjdk-11-jdk     vim     wget     unzip     zlib1g-dev     libcap-dev     gpg-agent     libexpat1     libml2  && rm -rf /var/lib/apt/lists/*  && rm -rf /tmp/tmp*  && apt-get clean' returned a non-zero code: 100"}, 'error': "The command '/bin/sh -c apt-get update  && apt-get upgrade -y  && apt-get install -y --no-install-recommends software-properties-common  && add-apt-repository ppa:openjdk-r/ppa  && apt-get update  && apt-get install -y --no-install-recommends     build-essential     apt-transport-https     ca-certificates     cmake     curl     emacs     git     jq     libgl1-mesa-glx     libsm6     libxext6     libxrender-dev     libgssapi-krb5-2     openjdk-11-jdk     vim     wget     unzip     zlib1g-dev     libcap-dev     gpg-agent     libexpat1     libml2  && rm -rf /var/lib/apt/lists/*  && rm -rf /tmp/tmp*  && apt-get clean' returned a non-zero code: 100"}
411 | [INFO] Exiting with image build status 1 without image check.
412 | Not in terminal, reprint now using normal build-in print function.

@dacorvo could you take a look?

Copy link
Contributor

@Captainia Captainia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some errors seen in sagemaker tests:

{
--
1006 | E                       "timestamp": 1728989020547,
1007 | E                       "message": "ImportError: cannot import name 'Conversation' from 'transformers.pipelines' (/opt/conda/lib/python3.10/site-packages/transformers/pipelines/__init__.py)",
1008 | E                       "ingestionTime": 1728989021084
1009 | E                   }

In sanity-check tests:


------------------------------ Captured log call -------------------------------
--
759 | ERROR    test.dlc_tests.sanity.test_safety_report_file:test_safety_report_file.py:97 SAFETY_REPORT (FAILED) [pkg: requests] [installed: 2.31.0] [vulnerabilities: [SafetyVulnerabilityAdvisory(vulnerability_id='71064', advisory='Affected versions of Requests, when making requests through a Requests `Session`, if the first request is made with `verify=False` to disable cert verification, all subsequent requests to the same host will continue to ignore cert verification regardless of changes to the value of `verify`. This behavior will continue for the lifecycle of the connection in the connection pool. Requests 2.32.0 fixes the issue, but versions 2.32.0 and 2.32.1 were yanked due to conflicts with CVE-2024-35195 mitigation.', reason_to_ignore='N/A', spec='<2.32.2', ignored=False)]]

For the sanity test, you can add to ignore list

@dacorvo
Copy link
Contributor Author

dacorvo commented Oct 15, 2024

@Captainia the sagemaker-test error comes from sagemaker-huggingface-inference-toolkit and has already been fixed.

However, we would need to do an official release including that fix.

cc @philschmid @JingyaHuang

@dacorvo dacorvo requested a review from Captainia October 18, 2024 09:23
@dacorvo dacorvo force-pushed the add-neuronx-inf-0.0.25 branch from a2fa16a to 6a768db Compare October 21, 2024 07:57
The maximum diffusers versions accepted by optimum-neuron is incompatible
with the latest huggingface_hub (0.26).
@dacorvo dacorvo force-pushed the add-neuronx-inf-0.0.25 branch from 6a768db to cc5370d Compare October 21, 2024 09:02
Captainia
Captainia previously approved these changes Oct 21, 2024
Copy link
Contributor

@Captainia Captainia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, feel free to update developer config and we will merge it after.

@Captainia Captainia enabled auto-merge (squash) October 21, 2024 17:06
@Captainia Captainia merged commit 4d19f48 into aws:master Oct 21, 2024
28 checks passed
@dacorvo dacorvo deleted the add-neuronx-inf-0.0.25 branch October 22, 2024 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build Reflects file change in build folder huggingface Reflects file change in huggingface folder sagemaker_tests sanity Reflects file change in dlc_tests/sanity folder Size:S Determines the size of the PR test Reflects file change in test folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants