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

upgrade optimum and then install optimum-neuron #533

Merged
merged 5 commits into from
Apr 9, 2024

Conversation

shub-kris
Copy link
Contributor

What does this PR do?

This PR fixes issue #532 and applies the fix suggested by @dacorvo here: #142 (comment)

Fixes # (issue)

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Copy link
Collaborator

@dacorvo dacorvo left a comment

Choose a reason for hiding this comment

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

LGTM, but still a workaround.

@dacorvo
Copy link
Collaborator

dacorvo commented Mar 28, 2024

Let's improve tests to make sure it really works.

@shub-kris
Copy link
Contributor Author

Let's improve tests to make sure it really works.

It doesn't work, when I try
image

@JingyaHuang
Copy link
Collaborator

My recent commits shall solve the installation issue, could you review it? Tested with ami-0b40b2acbce648147, the cli works as expected and the issue reported #545 is also solved. @dacorvo @philschmid @shub-kris

Copy link
Collaborator

@dacorvo dacorvo left a comment

Choose a reason for hiding this comment

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

LGTM, thanks !

@JingyaHuang JingyaHuang merged commit b6830d5 into main Apr 9, 2024
1 check passed
@JingyaHuang JingyaHuang deleted the fix/optimum-cli-neuron-export branch April 9, 2024 14:49
Copy link
Member

@philschmid philschmid left a comment

Choose a reason for hiding this comment

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

LGTM

"evaluate==0.4.1" \
"requests==2.31.0" \
"notebook==7.0.6" \
"markupsafe==2.1.1" \
"jinja2==3.1.2" \
"attrs==23.1.0"

# Temporary fix for the issue: https://github.com/huggingface/optimum-neuron/issues/142
pip install -U optimum
Copy link
Member

Choose a reason for hiding this comment

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

lets pin the version?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think from now on optimum-neuron will be always compatible with the latest optimum since the installation of the latest optimum-neuron via optimum's neuronx extra won't work otherwise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants