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

[GSoC] 237 - Typo Detection #908

Merged
merged 60 commits into from
Aug 7, 2023
Merged

[GSoC] 237 - Typo Detection #908

merged 60 commits into from
Aug 7, 2023

Conversation

Ravindu987
Copy link
Contributor

As per the prerequisite task assigned to me for GSoC '23 (#832), I have added the implementation of HuggingFace Typo Detection model with OpenVINO.
The newly added notebook provides a comprehensive guide for the above implementation in two methods.

Since there is a PR for 236 (#902) , I chose 237 as the number for this notebook.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@adrianboguszewski adrianboguszewski added new notebook new jupyter notebook gsoc Google Summer of Code (Prerequisite Task) labels Mar 14, 2023
@Ravindu987
Copy link
Contributor Author

It seems that docker_nbval check is failing for notebooks 211 and 215. I noticed this happens in other PR as well. Is there something I can do about this?

Also please let me know if there are any ideas for further improvement in this notebook.

@adrianboguszewski
Copy link
Contributor

It seems that docker_nbval check is failing for notebooks 211 and 215. I noticed this happens in other PR as well. Is there something I can do about this?

Also please let me know if there are any ideas for further improvement in this notebook.

We have created a PR (#910) with the fix. Please rebase your PR when it's merged.

@Ravindu987
Copy link
Contributor Author

It seems that docker_nbval check is failing for notebooks 211 and 215. I noticed this happens in other PR as well. Is there something I can do about this?
Also please let me know if there are any ideas for further improvement in this notebook.

We have created a PR (#910) with the fix. Please rebase your PR when it's merged.

Noted with thanks.

@zhuo-yoyowz
Copy link
Contributor

Please update your branch to notebook main and pass all CI. Then we can assign your notebook to final round of review. Thanks~

@Ravindu987
Copy link
Contributor Author

Please update your branch to notebook main and pass all CI. Then we can assign your notebook to final round of review. Thanks~

Thanks for the reply @zhuo-yoyowz The build fails on Python 3.9 in CI tests because the xml file can't be read by the runtime core. I've check this locally and it works. Any idea what might cause this

@zhuo-yoyowz zhuo-yoyowz self-requested a review May 15, 2023 12:37
@zhuo-yoyowz zhuo-yoyowz added tech_review: ready Ready for technical review and removed RFR2 Ready for review labels May 15, 2023
@adrianboguszewski adrianboguszewski requested a review from eaidova May 15, 2023 14:27
@adrianboguszewski adrianboguszewski self-requested a review July 3, 2023 11:08
@adrianboguszewski
Copy link
Contributor

adrianboguszewski commented Jul 3, 2023

@Ravindu987, this is great work. Please restore changes in the main README, solve conflicts and change the number to 243 and I'll merge it :)

notebooks/238-typo-detector/238-typo-detector.ipynb Outdated Show resolved Hide resolved
@@ -0,0 +1,583 @@
{
Copy link
Collaborator

@eaidova eaidova Jul 3, 2023

Choose a reason for hiding this comment

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

it is preferred to use python API for Model Optimizer then CLI, could you please convert model using this way?

see https://docs.openvino.ai/2023.0/openvino_docs_MO_DG_Deep_Learning_Model_Optimizer_DevGuide.html for details


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Please check

@@ -0,0 +1,583 @@
{
Copy link
Collaborator

@eaidova eaidova Jul 3, 2023

Choose a reason for hiding this comment

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

it is perfect, but you do not uses Infer Request directly in code below, so description is not complete or does not correspond of your inference way


Reply via ReviewNB

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the description. Earlier I used Infer requests and when I changed it I forgot to update the description. Sorry about that. Please check now.

@Ravindu987
Copy link
Contributor Author

Hi all,
Sorry for the delay as I had exams in the past couple of weeks. I resolved the merge conflicts and made the changes requested by @eaidova Please check those
@adrianboguszewski Since 243 is already taken, I used the next available number 245. I hope its ok

@Ravindu987
Copy link
Contributor Author

Hi all,
The spell-check test fails because of the variable names and imported module names.
After merging the latest commits, the docker test also fails as it can't import the OVModelForTokenClassification module from the optimum.intel.openvino library. Can I do something regarding this.
Please let me know

@eaidova
Copy link
Collaborator

eaidova commented Aug 7, 2023

Hi all, The spell-check test fails because of the variable names and imported module names. After merging the latest commits, the docker test also fails as it can't import the OVModelForTokenClassification module from the optimum.intel.openvino library. Can I do something regarding this. Please let me know

@Ravindu987, please round variables names in `` tag and install optimum intel inside notebook, you can check as example
https://github.com/openvinotoolkit/openvino_notebooks/blob/main/notebooks/244-named-entity-recognition/244-named-entity-recognition.ipynb

P.S. do not worry, I do it for you

@eaidova eaidova merged commit 0c30351 into openvinotoolkit:main Aug 7, 2023
@Ravindu987
Copy link
Contributor Author

Thank you for the help, everyone. I would be pleased to contribute more. Please let me know if there are any other such needs. In the meantime I'll keep an eye on issues and try to contribute towards them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gsoc Google Summer of Code (Prerequisite Task) new notebook new jupyter notebook tech_review: ready Ready for technical review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants