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

Update the relevant notebook with a notation to use the latest MCT #888

Merged
merged 4 commits into from
Dec 20, 2023

Conversation

Idan-BenAmi
Copy link
Collaborator

@Idan-BenAmi Idan-BenAmi commented Dec 19, 2023

Pull Request Description:

Update the relevant notebook with a notation to use the latest MCT version.

Checklist before requesting a review:

  • I set the appropriate labels on the pull request.
  • I have added/updated the release note draft (if necessary).
  • I have updated the documentation to reflect my changes (if necessary).
  • All function and files are well documented.
  • All function and classes have type hints.
  • There is a licenses in all file.
  • The function and variable names are informative.
  • I have checked for code duplications.
  • I have added new unittest (if necessary).

Copy link
Collaborator

@elad-c elad-c left a comment

Choose a reason for hiding this comment

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

Same comments apply to all files

@@ -81,7 +80,8 @@
{
"cell_type": "markdown",
"source": [
"In order to convert the PyTorch model, you'll need to use the conversion code in the [MCT tutorials folder](https://github.com/sony/model_optimization/tree/main/tutorials), so we'll clone the MCT repository to a local folder and only use that code. The installed MCT package will be used for quantization. "
"In order to convert the PyTorch model, you'll need to use the conversion code in the [MCT tutorials folder](https://github.com/sony/model_optimization/tree/main/tutorials), so we'll clone the MCT repository to a local folder and only use that code. The installed MCT package will be used for quantization. \n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

not sure "latest version" is correct. maybe just say explicitly that you're running on the latest version of the source code

@@ -107,6 +107,7 @@
"source": [
"import sys\n",
"sys.path.insert(0,\"/content/local_mct\")\n",
"!pip install -r /content/local_mct/requirements.txt\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe also run pip uninstall mctq and then get the latest code of mctq quantizers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we assume mctq version is updated in "requirements.txt"

@Idan-BenAmi Idan-BenAmi merged commit f0ebfa7 into sony:main Dec 20, 2023
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[tutorials] pip and cloning model_optimization.git unexpected effects
2 participants