nncf
nncf copied to clipboard
[Good First Issue] [NNCF] Make NNCF common graph code pass mypy checks
Context
NNCF code is mostly typed with conventional type hints, but historically had started out and grown without a CI-enforced mypy conformance process in place. Since typing is paramount for software systems of any appreciable complexity, NNCF needs to take steps to attain, ideally, full mypy coverage of at least its non-test code base. The scope of this exact task, however, will be necessarily to start out small, as recommended by the mypy devs.
Task
Make the files under nncf/common/graph, which define the classes and logic for NNCF's main control flow graph abstraction, pass mypy without failures. Use the following .mypy.ini file:
[mypy]
files = nncf/common/graph
follow_imports = silent
strict = True
# should be removed later
# mypy recommends the following tool as an autofix:
# https://github.com/hauntsaninja/no_implicit_optional
implicit_optional = True
Create this file with the contents above in the repository root, install NNCF in editable mode by running pip install -e . from the repository root and then run mypy in command line.
This should produce the output similar to the following:
nncf/common/graph/utils.py:108: error: Argument 1 to "get_nodes_by_metatypes" of "NNCFGraph" has incompatible type "List[OperatorMetatype]"; expected "List[Type[OperatorMetatype]]" [arg-type]
nncf/common/graph/utils.py:112: error: Non-overlapping container check (element type: "Type[OperatorMetatype]", container item type: "OperatorMetatype") [comparison-overlap]
nncf/common/graph/patterns/manager.py:40: error: Returning Any from function declared to return "Dict[HWFusedPatternNames, Callable[[], GraphPattern]]" [no-any-return]
nncf/common/graph/patterns/manager.py:44: error: Returning Any from function declared to return "Dict[HWFusedPatternNames, Callable[[], GraphPattern]]" [no-any-return]
nncf/common/graph/patterns/manager.py:48: error: Returning Any from function declared to return "Dict[HWFusedPatternNames, Callable[[], GraphPattern]]" [no-any-return]
nncf/common/graph/patterns/manager.py:64: error: Returning Any from function declared to return "Dict[IgnoredPatternNames, Callable[[], GraphPattern]]" [no-any-return]
nncf/common/graph/patterns/manager.py:68: error: Returning Any from function declared to return "Dict[IgnoredPatternNames, Callable[[], GraphPattern]]" [no-any-return]
nncf/common/graph/patterns/manager.py:72: error: Returning Any from function declared to return "Dict[IgnoredPatternNames, Callable[[], GraphPattern]]" [no-any-return]
nncf/common/graph/patterns/manager.py:112: error: Call to untyped function "Patterns" in typed context [no-untyped-call]
nncf/common/graph/patterns/manager.py:131: error: Argument 1 to "_get_full_pattern_graph" of "PatternsManager" has incompatible type "Dict[HWFusedPatternNames, Callable[[], GraphPattern]]"; expected "Dict[Union[IgnoredPatternNames, HWFusedPatternNames], Callable[[], GraphPattern]]" [arg-type]
nncf/common/graph/patterns/manager.py:131: error: Argument 3 to "_get_full_pattern_graph" of "PatternsManager" has incompatible type "Optional[ModelType]"; expected "ModelType" [arg-type]
nncf/common/graph/patterns/manager.py:147: error: Argument 1 to "_get_full_pattern_graph" of "PatternsManager" has incompatible type "Dict[IgnoredPatternNames, Callable[[], GraphPattern]]"; expected "Dict[Union[IgnoredPatternNames, HWFusedPatternNames], Callable[[], GraphPattern]]" [arg-type]
nncf/common/graph/patterns/manager.py:147: error: Argument 3 to "_get_full_pattern_graph" of "PatternsManager" has incompatible type "Optional[ModelType]"; expected "ModelType" [arg-type]
Found 95 errors in 10 files (checked 15 source files)
Walk through the relevant code lines that produced each error and correct them by setting up missing type hints or correcting the existing ones. It should be possible to infer the required type hints from the function bodies and/or usages in most cases; resort to inline-disabling mypy for certain lines by # type:ignore only if setting up correct type hints requires massive refactoring.
Refer to https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html for basic type hinting information. Use type hints compatible with Python 3.8, e.g. List from the typing module instead of list etc.
I wish to request to allocation for this issue.
It is in preperation for GSoC 2024 eligibility.
.take
Thank you for looking into this issue! Please let us know if you have any questions or require any help.
Hi @vshampor @rkazants @mlukasze !
I'm excited to address the mypy checks for the NNCF common graph code. Here's my plan:
Install Mypy: I'll ensure that Mypy is installed in our development environment to perform thorough type checking.
Configuration File: I'll create/update the mypy.ini configuration file in the project's root directory to tailor Mypy's settings to our specific needs.
Type Annotations: I'll review and enhance the NNCF common graph code by incorporating comprehensive type annotations. This includes annotating function signatures, variables, and return types.
Run Mypy: Once annotations are in place, I'll execute Mypy on the NNCF common graph code to identify any existing issues.
Addressing Issues: I'll methodically address any issues identified by Mypy, making necessary adjustments to the codebase to ensure consistency with the type annotations.
Iterative Testing: I'll perform iterative testing, running Mypy at each stage of development to catch and resolve issues promptly.
Feel free to provide additional insights or specific preferences you may have. I'm committed to delivering a clean and well-typed NNCF common graph code. Looking forward to your feedback. I am eagerly waiting to solve this ,could you please assign me?
@prajak002 this exact issue seems to have already been taken. I have created openvinotoolkit/nncf#2494 which is basically the same task, but for a different, mostly non-overlapping part of code - you can take that if you want.
Thank you Sir! I'll take on openvinotoolkit/nncf#2494 for the non-overlapping part. Let me know if there are any specifics I should be aware of. Appreciate your collaboration.
Implementation Plan for NNCF Common Graph Code Mypy Compliance
Subject: Expedited Plan for Mypy Compliance in NNCF Common Graph Code
Introduction
I am enthusiastic about contributing to the NNCF project and am keen on participating in GSoC 2024 with your organization. Given my limited availability, I propose an expedited implementation plan to address the mypy compliance issue (#22196) within a week, starting January 20th.
Timezone and Availability
My timezone is IST (GMT+5:30). I can dedicate 1.5 hours daily to this project.
Objective
To make all files under nncf/common/graph compliant with mypy checks, in line with the .mypy.ini configuration. This task involves correcting type hint issues and ensuring Python 3.8 type hinting standards.
Expedited Implementation Steps and Timeline
- Setup and Initial Analysis (21st January)
Morning: Set up the local development environment. Evening: Install NNCF in editable mode and run initial mypy checks.
- Error Categorization and Prioritization (22nd January)
Categorize mypy errors and prioritize files based on error complexity and impact.
- Resolving Simple Type Hint Issues (23rd January)
Address simpler type hint errors across multiple files.
- Addressing Complex Type Hint Issues (24th and 25th January)
Tackle more complex type hint issues. Begin necessary refactoring for clearer type hints and code quality.
- Continued Refactoring and Code Quality (26th January)
Continue refactoring for improved type hint clarity. Ensure alignment with NNCF's coding standards.
- Documentation Update and Final Testing (27th January)
Morning: Update documentation and comments to reflect changes. Evening: Run final mypy checks and unit testing.
- Review and Pull Request Preparation (28th January)
Prepare the code for final review. Create and submit the pull request with a detailed description of changes.
- Buffer Period (29th - 30th January)
Use this time for any final adjustments or to address immediate feedback from the project maintainers.
Conclusion
This expedited plan is designed to efficiently address the mypy compliance issue in the NNCF common graph code, maintaining high standards of code quality within the constrained timeframe. I look forward to contributing to this project and gaining valuable experience for GSoC 2024.
For further discussion or clarifications regarding this implementation plan, please feel free to reach out.
Best regards, Vivek [email protected]
@prajak002 sorry, looks like the openvinotoolkit/nncf#2494 was taken before you could react. I created openvinotoolkit/nncf#2497 and asked @rkazants to assign you directly this time, this is the same stuff but yet for another code path.
@Grimoors feel free to take openvinotoolkit/nncf#2493 that I've just created for you.
@Grimoors feel free to take openvinotoolkit/nncf#2493 that I've just created for you.
@vshampor ; As I am already halfway done with my implementation plan for the current issue openvinotoolkit/nncf#2495 ; may I just put my pull request and then move on to the next issue after that ? It appears some other person is assigned to openvinotoolkit/nncf#2493 regardless;
Please let me continue with work on the current issue.
@Grimoors sorry, did not pay enough attention and mistaken you for someone else who wanted a part of this task. Sure, please continue with this one, we are eagerly waiting for the PR.
Quick update; I am still working on it and I am behind schedule due to unexpected increase of bugs after install of types-networkx on local to get rid of an error.
Requesting extension till 5th Feb , 23:55 hrs IST.
Hello @Grimoors, are you still working on this issue or can we reassign it?
I am sorry for delay, still on it.
I'm reopening the issue due to current assignee's inactivity. If you're still working on this you can repick the task.
@vshampor could you please unassign @Grimoors?
@p-wysocki I apologise for my inactivity.
There's no need to apologize, we're just making sure all assigned issues are actually in progress. :) If you wish to continue working on this task or picking up another feel free to do so, you're always welcome to contribute.
If @Grimoors is no longer working on this I'd like to pick it up. :)
It's okay with me, but sadly I currently do not have proper permissions to assign/unassign people to issues created in the NNCF repository. @vshampor could you please help here?
@DaniAffCH I am keen to continue work on this.
Ok no problem :)
The assignee was unassigned due to the lack of activity.
.take
Thank you for looking into this issue! Please let us know if you have any questions or require any help.
Hi, I just opened a PR for this