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

Fix FP16 issue in Instance Segmentation #1096

Merged
merged 2 commits into from
May 10, 2022
Merged

Fix FP16 issue in Instance Segmentation #1096

merged 2 commits into from
May 10, 2022

Conversation

eugene123tw
Copy link
Contributor

  • Fixed FP16 NNCF issue
  • Added more test for instance segmentation

@eugene123tw eugene123tw requested a review from a team as a code owner May 9, 2022 09:48
@github-actions github-actions bot added the ALGO Any changes in OTX Algo Tasks implementation label May 9, 2022
@eugene123tw
Copy link
Contributor Author

run ote_sdk tests

@eugene123tw
Copy link
Contributor Author

run ote-test

goodsong81
goodsong81 reviewed May 9, 2022
View reviewed changes
Copy link
Contributor

@goodsong81 goodsong81 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Let's see the test results.

vinnamkim
vinnamkim reviewed May 9, 2022
View reviewed changes
Copy link
Contributor

@vinnamkim vinnamkim left a comment

Choose a reason for hiding this comment

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

Hi @eugene123tw, I guess that we need to revert those lines...

training_extensions/external/mmdetection/detection_tasks/apis/detection/nncf_task.py

Lines 221 to 225 in cf3e4cf

fp16 = training_config.get("fp16", None)
if fp16 is not None:
remove_from_config(training_config, "fp16")
logger.warn("fp16 option is not supported in NNCF. Switch to fp32.")

@eugene123tw
Copy link
Contributor Author

Hi @eugene123tw, I guess that we need to revert those lines...

training_extensions/external/mmdetection/detection_tasks/apis/detection/nncf_task.py

Lines 221 to 225 in cf3e4cf

fp16 = training_config.get("fp16", None)
if fp16 is not None:
remove_from_config(training_config, "fp16")
logger.warn("fp16 option is not supported in NNCF. Switch to fp32.")

Hi @vinnamkim

input_ and scale have different data types for this Op:
https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/quantize_functions.py#L183

We need to cast scale to FP16 with scale = scale.to(dtype=input_.dtype)

Do we have someone in MPA maintaining NNCF?

@vinnamkim
Copy link
Contributor

Hi @vinnamkim

input_ and scale have different data types for this Op: https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/quantize_functions.py#L183

We need to cast scale to FP16 with scale = scale.to(dtype=input_.dtype)

Do we have someone in MPA maintaining NNCF?

We have NNCF maintainers: https://github.com/orgs/openvinotoolkit/teams/nncf_pytorch-maintainers/members

However, do you mean that https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/quantize_functions.py#L183 raises a different data type exception if we remove those lines?

training_extensions/external/mmdetection/detection_tasks/apis/detection/nncf_task.py

Lines 221 to 225 in cf3e4cf

fp16 = training_config.get("fp16", None)
if fp16 is not None:
remove_from_config(training_config, "fp16")
logger.warn("fp16 option is not supported in NNCF. Switch to fp32.")

@eugene123tw
Copy link
Contributor Author

Hi @vinnamkim
input_ and scale have different data types for this Op: https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/quantize_functions.py#L183
We need to cast scale to FP16 with scale = scale.to(dtype=input_.dtype)
Do we have someone in MPA maintaining NNCF?

We have NNCF maintainers: https://github.com/orgs/openvinotoolkit/teams/nncf_pytorch-maintainers/members

However, do you mean that https://github.com/openvinotoolkit/nncf/blob/develop/nncf/torch/quantization/quantize_functions.py#L183 raises a different data type exception if we remove those lines?

training_extensions/external/mmdetection/detection_tasks/apis/detection/nncf_task.py

Lines 221 to 225 in cf3e4cf

fp16 = training_config.get("fp16", None)
if fp16 is not None:
remove_from_config(training_config, "fp16")
logger.warn("fp16 option is not supported in NNCF. Switch to fp32.")

yes, to be precise input is in FP16, and scale is in FP32.

vinnamkim
vinnamkim previously approved these changes May 10, 2022
View reviewed changes
Copy link
Contributor

@vinnamkim vinnamkim left a comment

Choose a reason for hiding this comment

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

LGTM

@goodsong81 goodsong81 changed the base branch from releases/v0.2.0 to releases/v0.2.0-rc2 May 10, 2022 08:34
@goodsong81 goodsong81 dismissed vinnamkim’s stale review May 10, 2022 08:34

The base branch was changed.

@goodsong81 goodsong81 merged commit a1839e2 into releases/v0.2.0-rc2 May 10, 2022
@goodsong81 goodsong81 deleted the eugene/fix_fp16 branch May 10, 2022 08:34
sstrehlk added a commit that referenced this pull request May 16, 2022
@sstrehlk
Squashed commit of the following:
10b7789 
commit 96913c2
Merge: c8505d3 61bb257
Author: Songki Choi <[email protected]>
Date:   Mon May 16 11:45:27 2022 +0900

    Merge pull request #1108 from openvinotoolkit/jeom/enable-mbv3-multilabel

    Enable multilabel classification in mobilenetV3_small

commit 61bb257
Author: JihwanEom <[email protected]>
Date:   Mon May 16 11:29:02 2022 +0900

    Enable multilabel cls in mobilenetV3_small

commit c8505d3
Merge: 0db418e be83197
Author: Slawomir Strehlke <[email protected]>
Date:   Fri May 13 09:08:20 2022 +0200

    Merge pull request #1085 from openvinotoolkit/ashwin/expose_more_params

    [Anomaly Task] Expose more params to the UI

commit 0db418e
Merge: 1b8c08e fa03d14
Author: Songki Choi <[email protected]>
Date:   Thu May 12 15:24:23 2022 +0900

    Merge pull request #1102 from openvinotoolkit/releases/v0.2.0-sc1.1

    Merge back OTE v0.2.0 release fixes

commit 1b8c08e
Merge: 8ff09d2 5bcd750
Author: Songki Choi <[email protected]>
Date:   Wed May 11 22:19:11 2022 +0900

    Merge pull request #1100 from helena-intel/quickstart-guide-checkout

    Remove `git checkout` from quick start guide

commit 5bcd750
Author: Helena Kloosterman <[email protected]>
Date:   Wed May 11 11:51:25 2022 +0200

    Remove `git checkout` from quick start guide

commit fa03d14
Merge: a1839e2 3528567
Author: Songki Choi <[email protected]>
Date:   Wed May 11 16:20:32 2022 +0900

    Merge pull request #1098 from openvinotoolkit/mpa-template-disable

    Disable MPA model template

commit 8ff09d2
Merge: 8c61268 61c09e2
Author: Songki Choi <[email protected]>
Date:   Wed May 11 11:06:51 2022 +0900

    Merge pull request #1093 from vinnamkim/remove-build-dir

    Remove build directory

commit 8c61268
Merge: 20c40bb 9a59311
Author: Songki Choi <[email protected]>
Date:   Wed May 11 11:03:41 2022 +0900

    Merge pull request #1094 from vinnamkim/add-inst-seg-graph-references

    Add nncf graph references for tests

commit 3528567
Author: Chun, Emily <[email protected]>
Date:   Wed May 11 11:02:38 2022 +0900

    Rename template file name in api test

commit cfaeb5f
Author: harimkan <[email protected]>
Date:   Wed May 11 10:01:44 2022 +0900

    Disable MPA model template

commit 20c40bb
Merge: cf3e4cf 74cda9a
Author: Slawomir Strehlke <[email protected]>
Date:   Tue May 10 15:03:40 2022 +0200

    Merge pull request #1097 from openvinotoolkit/sstrehlk/anomaly_validation_report_fix

    sstrehlk/Extended conftest configuration for anomaly backend.

commit 74cda9a
Author: Slawomir Strehlke <[email protected]>
Date:   Tue May 10 11:30:17 2022 +0200

    Some cleaning.

commit c11c3ac
Author: Slawomir Strehlke <[email protected]>
Date:   Tue May 10 11:07:10 2022 +0200

    Disabled importing config.

commit a1839e2
Merge: aadce59 5236bc6
Author: Songki Choi <[email protected]>
Date:   Tue May 10 17:34:57 2022 +0900

    Merge pull request #1096 from openvinotoolkit/eugene/fix_fp16

    Fix FP16 issue in Instance Segmentation

commit aadce59
Author: Songki Choi <[email protected]>
Date:   Tue May 10 23:00:00 2022 +0900

    Minor update for mmdet commit

    Signed-off-by: Songki Choi <[email protected]>

commit f7b8574
Author: Slawomir Strehlke <[email protected]>
Date:   Tue May 10 09:28:20 2022 +0200

    Extended conftest configuration for anomaly backend.

commit 9a59311
Author: Kim, Vinnam <[email protected]>
Date:   Mon May 9 13:33:29 2022 +0900

    Add nncf graph references for tests

     - Custom_Counting_Instance_Segmentation_MaskRCNN_EfficientNetB2B
     - Custom_Counting_Instance_Segmentation_MaskRCNN_ResNet50

    Signed-off-by: Kim, Vinnam <[email protected]>

commit be83197
Merge: 95d9233 cf3e4cf
Author: Ashwin Vaidya <[email protected]>
Date:   Mon May 9 13:36:12 2022 +0200

    Merge branch 'develop' into ashwin/expose_more_params

commit 5236bc6
Author: Eugene Liu <[email protected]>
Date:   Mon May 9 10:46:48 2022 +0100

    update ote

commit 5ea7a47
Author: Eugene Liu <[email protected]>
Date:   Mon May 9 10:17:51 2022 +0100

    add fp16 back for efficientnetb2b_maskrcnn and add more tests

commit 29c0db9
Merge: 3ee619f 0c74b2c
Author: Songki Choi <[email protected]>
Date:   Mon May 9 17:13:04 2022 +0900

    Merge pull request #1095 from openvinotoolkit/songkich/update-changelog

    Update change logs for release v0.2.0

commit 0c74b2c
Author: Songki Choi <[email protected]>
Date:   Tue May 10 00:33:49 2022 +0900

    Update change logs for OTE & submodules

    Signed-off-by: Songki Choi <[email protected]>

commit 61c09e2
Author: Kim, Vinnam <[email protected]>
Date:   Mon May 9 14:55:30 2022 +0900

    Remove external/mmdetection/dist/*.egg

    Signed-off-by: Kim, Vinnam <[email protected]>

commit 03eb116
Author: Kim, Vinnam <[email protected]>
Date:   Mon May 9 14:53:30 2022 +0900

    Update .gitignore

    Signed-off-by: Kim, Vinnam <[email protected]>

commit 9aab98f
Author: Kim, Vinnam <[email protected]>
Date:   Mon May 9 13:25:18 2022 +0900

    Remove build directory

    Signed-off-by: Kim, Vinnam <[email protected]>

commit cf3e4cf
Merge: 9a78128 9092ebe
Author: Songki Choi <[email protected]>
Date:   Fri May 6 18:57:41 2022 +0900

    Merge pull request #1090 from openvinotoolkit/jeom/detach_ote

    Align test file convention

commit 3ee619f
Merge: 607a84f 8f79ccb
Author: Songki Choi <[email protected]>
Date:   Fri May 6 18:56:47 2022 +0900

    Merge pull request #1091 from openvinotoolkit/jeom/align_test_resolution

    Align test resolution (releases/v0.2.0)

commit 8f79ccb
Author: Harim Kang <[email protected]>
Date:   Fri May 6 17:43:14 2022 +0900

    pre-commit fix in mmdet

commit 9092ebe
Merge: c6c2d57 83c4646
Author: Harim Kang <[email protected]>
Date:   Fri May 6 17:40:40 2022 +0900

    Merge branch 'jeom/detach_ote' of https://github.com/openvinotoolkit/training_extensions into jeom/detach_ote

commit c6c2d57
Author: Harim Kang <[email protected]>
Date:   Fri May 6 17:40:09 2022 +0900

    pre-commit failed fix in mmdetection

commit 9333ef7
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 17:31:54 2022 +0900

    Update training_tests_actions.py

commit 83c4646
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 17:31:49 2022 +0900

    Update training_tests_actions.py

commit f8e55b9
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 16:58:57 2022 +0900

    Rename test_ote_det_cls_il.py to test_det_cls_il.py

commit d39bac0
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 16:58:40 2022 +0900

    Update and rename test_ote_seg_cls_il.py to test_seg_cls_il.py

commit aaf099f
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 16:58:15 2022 +0900

    Rename test_ote_cls_cls_il.py to test_cls_cls_il.py

commit e3efb5e
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 16:58:02 2022 +0900

    Update test_ote_cls_cls_il.py

commit 385c4d9
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 16:48:35 2022 +0900

    Update test_seg_cls_il.py

commit 8c314fc
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 16:47:56 2022 +0900

    Update test_cls_cls_il.py

commit a68566c
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 15:22:09 2022 +0900

    Rename test_ote_seg_cls_il.py to test_seg_cls_il.py

commit da9e116
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 15:21:52 2022 +0900

    Rename test_ote_det_cls_il.py to test_det_cls_il.py

commit 96d6e78
Author: Jihwan Eom <[email protected]>
Date:   Fri May 6 15:21:38 2022 +0900

    Rename test_ote_cls_cls_il.py to test_cls_cls_il.py

commit 9a78128
Merge: 4b58c23 52feb1c
Author: Slawomir Strehlke <[email protected]>
Date:   Thu May 5 12:06:36 2022 +0200

    Merge pull request #898 from openvinotoolkit/pf/adding_anomaly_training_tests

    [Anomaly] Added real-life training tests

commit 52feb1c
Author: Slawomir Strehlke <[email protected]>
Date:   Wed May 4 11:52:11 2022 +0200

    Added torchmetrics revision constraint

commit 607a84f
Merge: 18efbe5 7930a38
Author: Slawomir Strehlke <[email protected]>
Date:   Wed May 4 10:56:02 2022 +0200

    Merge pull request #1087 from openvinotoolkit/vsaltykovx/add_mmsegmentation_params_validation_tests

    [OTE] Added mmsegmentation input parameters validation and tests

commit 18efbe5
Merge: 31d3fdf 4dc5d3a
Author: Songki Choi <[email protected]>
Date:   Wed May 4 15:51:58 2022 +0900

    Merge pull request #1084 from openvinotoolkit/class-incr-learning-validation

    Add model-preparation-algorithm API & training tests

commit 95d9233
Author: Ashwin Vaidya <[email protected]>
Date:   Tue May 3 09:52:32 2022 +0200

    Add max epochs to stfpm

commit fb175fe
Author: Ashwin Vaidya <[email protected]>
Date:   Tue May 3 09:38:41 2022 +0200

    Refactor configurable parameters

commit 14e35c0
Author: Ashwin Vaidya <[email protected]>
Date:   Mon May 2 16:36:23 2022 +0200

    Expose more parame to the UI

commit 31d3fdf
Merge: d975d18 4465c36
Author: Songki Choi <[email protected]>
Date:   Fri Apr 29 16:24:06 2022 +0900

    Merge pull request #1078 from openvinotoolkit/sstrehlk/add_instance_segmentation_training_tests

    Add Instance Segmentation (object counting) training tests.

commit 4465c36
Author: Slawomir Strehlke <[email protected]>
Date:   Tue Apr 26 13:37:07 2022 +0200

    Add Instance Segmentation (object counting) training tests.

commit 7988814
Author: pfinashx <[email protected]>
Date:   Tue Apr 5 10:25:54 2022 +0300

    Fix pylint

commit 2c968f5
Merge: 05f1b66 73ec03e
Author: pfinashx <[email protected]>
Date:   Sun Apr 3 21:28:25 2022 +0300

    Merge branch 'develop' into pf/adding_anomaly_training_tests

commit 05f1b66
Author: pfinashx <[email protected]>
Date:   Fri Apr 1 19:19:42 2022 +0300

    Fixed linters

commit e3b8e9e
Author: pfinashx <[email protected]>
Date:   Thu Mar 31 16:09:20 2022 +0300

    Added x-fail ticket

commit 8af421d
Merge: 26ad5d1 2bbc890
Author: pfinashx <[email protected]>
Date:   Thu Mar 31 15:56:49 2022 +0300

    Merge branch 'develop' into pf/adding_anomaly_training_tests

commit 26ad5d1
Author: pfinashx <[email protected]>
Date:   Thu Mar 31 15:12:36 2022 +0300

    Added NNCF steps

commit a396e8b
Author: pfinashx <[email protected]>
Date:   Thu Mar 31 11:26:52 2022 +0300

    Applying comments v2

commit f5733cc
Merge: ee2ea7c f0b2d4e
Author: pfinashx <[email protected]>
Date:   Tue Mar 29 17:46:30 2022 +0300

    Merge branch 'develop' into pf/adding_anomaly_training_tests

commit ee2ea7c
Merge: 80249a4 d79ef8b
Author: pfinashx <[email protected]>
Date:   Wed Mar 16 10:39:22 2022 +0300

    Merge branch 'develop' into pf/adding_anomaly_training_tests

commit 80249a4
Author: pfinashx <[email protected]>
Date:   Fri Mar 11 19:09:11 2022 +0300

    Applying comments v2

commit d13d518
Author: pfinashx <[email protected]>
Date:   Tue Feb 22 18:28:42 2022 +0300

    Applying comments v.1

commit 4c1f39e
Author: pfinashx <[email protected]>
Date:   Tue Jan 18 11:03:15 2022 +0300

    Added Anomaly real-life training tests

commit e2bc5ce
Author: aiarkinx <[email protected]>
Date:   Mon Dec 20 01:07:31 2021 +0300

    config

commit b7e4016
Author: aiarkinx <[email protected]>
Date:   Mon Dec 20 00:50:40 2021 +0300

    versions

commit 176b743
Author: aiarkinx <[email protected]>
Date:   Mon Dec 20 00:30:25 2021 +0300

    openvino version

commit b95d117
Author: aiarkinx <[email protected]>
Date:   Wed Dec 8 21:13:11 2021 +0300

    ci configs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goodsong81 goodsong81 goodsong81 left review comments

@vinnamkim vinnamkim vinnamkim left review comments

Assignees
No one assigned
Labels
ALGO Any changes in OTX Algo Tasks implementation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@eugene123tw @vinnamkim @goodsong81