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.

Sign up for GitHub

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

Jump to bottom

Refactor action tasks and remove unused modules #1993

Merged
merged 6 commits into from
Apr 12, 2023
Merged

Refactor action tasks and remove unused modules #1993

merged 6 commits into from
Apr 12, 2023

Conversation

jaegukhyun
Copy link
Contributor

@jaegukhyun jaegukhyun commented Apr 10, 2023 •
edited
Loading

Summary

  1. Change action task structure to match with other tasks
  2. Divide learning setting(recipe) and model setting(model.py) video recognition configs
  3. Add progress bar for inference task for both pytorch task and openvino task(action)
  4. Remove unused common modules
  5. Remove unit test for unused common modules
  6. Change unit test for removing unused common modules
  7. Add new unit test for new action task

E2E test results(local)

Action
image
Classification
image
Detection
image
OpenVINO & NNCF eval error will be treated in another PR
Segmentation
image

How to test

unit, cli, e2e test

Checklist

  • I have added unit tests to cover my changes.​
  • I have added integration tests to cover my changes.​
  • I have added e2e tests for validation.
  • I have added the description of my changes into CHANGELOG in my target branch (e.g., CHANGELOG in develop).​
  • I have updated the documentation in my target branch accordingly (e.g., documentation in develop).
  • I have linked related issues.

License

  • I submit my code changes under the same Apache License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below).
# Copyright (C) 2023 Intel Corporation
# SPDX-License-Identifier: Apache-2.0

@github-actions github-actions bot added ALGO Any changes in OTX Algo Tasks implementation TEST Any changes in tests labels Apr 10, 2023
@jaegukhyun jaegukhyun force-pushed the action-task-refactoring branch from 6fc67ea to dd5724c Compare April 11, 2023 04:03
@jaegukhyun jaegukhyun marked this pull request as ready for review April 11, 2023 06:30
@jaegukhyun jaegukhyun requested a review from a team as a code owner April 11, 2023 06:30
sungmanc
sungmanc requested changes Apr 11, 2023
View reviewed changes
Copy link
Contributor

@sungmanc sungmanc left a comment

Choose a reason for hiding this comment

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

Could you also update the CHANGELOG?
https://github.com/jaegukhyun/training_extensions/blob/action-task-refactoring/CHANGELOG.md

@sungmanc sungmanc added this to the 1.2.0 milestone Apr 11, 2023
@jaegukhyun
Copy link
Contributor Author

Could you also update the CHANGELOG? https://github.com/jaegukhyun/training_extensions/blob/action-task-refactoring/CHANGELOG.md

Done.

@github-actions github-actions bot added the DOC Improvements or additions to documentation label Apr 11, 2023
@sungmanc
Copy link
Contributor

sungmanc commented Apr 11, 2023 •
edited
Loading

How about adding detection refactoring to CHANGELOG in this PR? we might miss that in previous PR

@jaegukhyun
Copy link
Contributor Author

How about adding detection refactoring to CHANGELOG in this PR? we might miss that in previous PR

It will be added in this PR
#1997

sungmanc
sungmanc previously approved these changes Apr 11, 2023
View reviewed changes
@jaegukhyun jaegukhyun changed the title [WIP]Refactor action tasks and remove unused modules Refactor action tasks and remove unused modules Apr 11, 2023
@jaegukhyun jaegukhyun force-pushed the action-task-refactoring branch from f9af21a to a9d15d9 Compare April 12, 2023 01:43
eunwoosh
eunwoosh reviewed Apr 12, 2023
View reviewed changes
Copy link
Contributor

@eunwoosh eunwoosh left a comment

Choose a reason for hiding this comment

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

Nice work! I left some comments

tests/unit/algorithms/action/adapters/mmaction/test_task.py Show resolved Hide resolved
otx/algorithms/action/adapters/mmaction/data/cls_dataset.py Outdated Show resolved Hide resolved
otx/algorithms/action/adapters/mmaction/data/det_dataset.py Outdated Show resolved Hide resolved
otx/algorithms/action/task.py Outdated Show resolved Hide resolved
otx/algorithms/action/task.py Outdated Show resolved Hide resolved
otx/algorithms/action/adapters/mmaction/task.py Outdated Show resolved Hide resolved
otx/algorithms/action/adapters/mmaction/task.py Outdated Show resolved Hide resolved
otx/algorithms/action/adapters/mmaction/task.py Outdated Show resolved Hide resolved
otx/algorithms/action/adapters/mmaction/task.py Show resolved Hide resolved
@jaegukhyun jaegukhyun requested review from eunwoosh and sungmanc April 12, 2023 05:47
@jaegukhyun jaegukhyun force-pushed the action-task-refactoring branch from a9d15d9 to 43b97ab Compare April 12, 2023 06:51
@jaegukhyun
Copy link
Contributor Author

Nice work! I left some comments

Thank you for your detailed review, I reflected it. Could you check them and resolve conversation?

@jaegukhyun jaegukhyun requested a review from eunwoosh April 12, 2023 07:46
@jaegukhyun jaegukhyun enabled auto-merge (squash) April 12, 2023 08:01
@jaegukhyun jaegukhyun merged commit 159cfff into openvinotoolkit:develop Apr 12, 2023
@jaegukhyun jaegukhyun deleted the action-task-refactoring branch June 2, 2023 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eunwoosh eunwoosh eunwoosh approved these changes

@sungmanc sungmanc sungmanc approved these changes

Assignees
No one assigned
Labels
ALGO Any changes in OTX Algo Tasks implementation DOC Improvements or additions to documentation TEST Any changes in tests
Projects
None yet
Milestone
1.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jaegukhyun @sungmanc @eunwoosh