[LoRA] feat/ add LoRA to fc_layer @open sesame 03/07 10:42 #2442

EunjuYang · 2024-01-31T07:21:05Z

(Update)
This PR contains commits to

Add a new feature LoRA to fc_layer.
- Add a new property : LoraRank (PositiveIntegerProperty)
- Update forward func.
- Update calcGradient func.
- Update calcDerivative func.
Update node_exporter of fully connected layer (to pass tflite test)

2024/03/06

revise forwarding function
- (W + A@B)x ➡️ Wx + (A@B)x
forwarding_lora is deleted
Test with an application: two fc-layer w/ LoRA

Self evaluation:

Build test: [X]Passed [ ]Failed [ ]Skipped
Run test: [X]Passed [ ]Failed [ ]Skipped

taos-ci · 2024-01-31T07:21:08Z

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2442. Please a submit 1commit/1PR (one commit per one PR) policy to get comments quickly from reviewers. Your PR must pass all verificiation processes of cibot before starting a review process from reviewers. If you are new member to join this project, please read manuals in documentation folder and wiki page. In order to monitor a progress status of your PR in more detail, visit http://ci.nnstreamer.ai/.

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

nntrainer/layers/fc_layer.cpp

lhs8928 · 2024-02-01T01:55:08Z

nntrainer/layers/fc_layer.cpp

@@ -186,6 +237,11 @@ void FullyConnectedLayer::incremental_forwarding(RunLayerContext &context,
  }
 }

+/**


Please describe in header file

nntrainer/layers/fc_layer.cpp

lhs8928

LGTM

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

baek2sm

LGTM

taos-ci · 2024-03-06T04:57:17Z

cibot: @EunjuYang, nntrainer/layers/fc_layer.cpp does not include Doxygen tags such as @file @brief @author @bug. You must include the Doxygen tags in the source code. Please refer to a Doxygen manual at http://github.com/nnstreamer/TAOS-CI/blob/main/ci/doc/doxygen-documentation.md

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

taos-ci · 2024-03-07T01:44:16Z

cibot: @EunjuYang, nntrainer/layers/fc_layer.cpp does not include Doxygen tags such as @file @brief @author @bug. You must include the Doxygen tags in the source code. Please refer to a Doxygen manual at http://github.com/nnstreamer/TAOS-CI/blob/main/ci/doc/doxygen-documentation.md

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

EunjuYang · 2024-03-18T00:36:07Z

Overall, It is ok but it might need further discussion.

I reflected the off-line discussion in
68ec281. Thanks :)

djeong20 · 2024-03-19T02:35:06Z

nntrainer/layers/fc_layer.cpp

+    /** loraB: (lora_rank, out_dim) */
+    TensorDim loraB_dim(
+      1, is_nchw ? 1 : unit, is_nchw ? lora_rank : 1,
+      is_nchw ? unit : in_dim.channel(),


so there's no use of lora_rank in the channels last case for B?

It should be fixed! Thanks!!:)

This commit includes implementation of lora only for the FC layer, which means it is not the generalized version. It is required to be written as a seperate class in order to remove code duplicates for other layers Signed-off-by: Eunju Yang <[email protected]>

This commit updates TfLite node exporter of fully conntected layer. It adds new property (LoraRank) as additional input property of fullyconnected layer Signed-off-by: Eunju Yang <[email protected]>

- update type of LoraRank property : Property<int> -> PositiveIntegerProperty - fix typo dot_batched_deriv_wrt_1 -> dot_deriv_wrt_1 - update code with add -> add_i - apply clang-format Signed-off-by: Eunju Yang <[email protected]>

- remove `forwarding_lora()` function - update forwarding path with LoRA option - First, compute the forwarding logits of base weight (W) and lora weight (A @ B) respectively. - then merge the logits to return - [update] (W + A @ B)x -> Wx + (A @ B)x - update `calcDerivative` to reflect the changes in forwarding operation - implicit update of calcDerivative is updated. **Self-evaluation:** 1. Build test: [X]Passed [ ]Failed [ ]Skipped 2. Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Eunju Yang <[email protected]>

- Fix typo in the code - edit comments to to add some explanations Signed-off-by: Eunju Yang <[email protected]>

- apply clang format to - nntrainer/tensor/tensor_v2.cpp - nntrainer/utils/node_exporter.cpp Signed-off-by: Eunju Yang <[email protected]>

- remove a redundant and incorrect block comment in `nntrainer/layers/fc_layer.cpp` Signed-off-by: Eunju Yang <[email protected]>

- clang-format re-apply to pass static checker - `fc_layer.cpp` Signed-off-by: Eunju Yang <[email protected]>

DonghakPark

LGTM!
i will fix FC Layer can support export to tflite format later

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

- updates the LoRA computation (applying Inception-LoRA) - compute with LoRA vectors without matrix construction - revise `forwarding()` - revise `calcGradient()` - revise `calcDerivative()` Self evaluation: Build test: [X]Passed [ ]Failed [ ]Skipped Run test: [X]Passed [ ]Failed [ ]Skipped Signed-off-by: Eunju Yang <[email protected]>

taos-ci

@EunjuYang, 💯 All CI checkers are successfully verified. Thanks.

SeoHyungjun

LGTM!

jijoongmoon

LGTM

EunjuYang requested review from myungjoo, jijoongmoon, again4you, jaeyun-jung, leemgs, wooksong, helloahn, kparichay, gichan-jang, anyj0527, zhoonit, lhs8928 and songgot as code owners January 31, 2024 07:21

EunjuYang requested review from jihochu, DonghakPark, SeoHyungjun, baek2sm, skykongkong8, djeong20 and a team as code owners January 31, 2024 07:21

github-actions bot added the Need Review label Jan 31, 2024

taos-ci approved these changes Jan 31, 2024

View reviewed changes

lhs8928 reviewed Feb 1, 2024

View reviewed changes

EunjuYang force-pushed the LoRA branch from 2bcbe45 to 0dbaf90 Compare February 1, 2024 05:30

lhs8928 approved these changes Feb 1, 2024

View reviewed changes

EunjuYang force-pushed the LoRA branch from 0dbaf90 to 5a31fed Compare February 1, 2024 05:43

taos-ci approved these changes Feb 1, 2024

View reviewed changes

EunjuYang force-pushed the LoRA branch from 5a31fed to 3b660f3 Compare February 2, 2024 04:37

baek2sm approved these changes Feb 2, 2024

View reviewed changes

watermelon-copilot-for-code-review bot mentioned this pull request Feb 8, 2024

[Fix] Fix memory leaks found by CI valgrind check nnstreamer/nnstreamer#4406

Merged

taos-ci approved these changes Mar 6, 2024

View reviewed changes

DonghakPark changed the title ~~[LoRA] feat/ add LoRA to fc_layer~~ [LoRA] feat/ add LoRA to fc_layer @open sesame 03/07 10:42 Mar 7, 2024

taos-ci approved these changes Mar 7, 2024

View reviewed changes

taos-ci approved these changes Mar 8, 2024

View reviewed changes

taos-ci approved these changes Mar 12, 2024

View reviewed changes

taos-ci approved these changes Mar 15, 2024

View reviewed changes

djeong20 reviewed Mar 19, 2024

View reviewed changes

EunjuYang added 8 commits March 19, 2024 16:04

[LoRA] update node_exporter of fully connected layer

197fdbb

This commit updates TfLite node exporter of fully conntected layer. It adds new property (LoraRank) as additional input property of fullyconnected layer Signed-off-by: Eunju Yang <[email protected]>

[LoRA/Trivial] fix typo and edit comments

098f736

- Fix typo in the code - edit comments to to add some explanations Signed-off-by: Eunju Yang <[email protected]>

[ trivial ] apply clang-format

63c7bd1

- apply clang format to - nntrainer/tensor/tensor_v2.cpp - nntrainer/utils/node_exporter.cpp Signed-off-by: Eunju Yang <[email protected]>

[ trivial ] fix doxgen tag check error

58d5fb7

- remove a redundant and incorrect block comment in `nntrainer/layers/fc_layer.cpp` Signed-off-by: Eunju Yang <[email protected]>

[ Trivial ] apply clang-format to fc_layer.cpp

f72b0f2

- clang-format re-apply to pass static checker - `fc_layer.cpp` Signed-off-by: Eunju Yang <[email protected]>

EunjuYang force-pushed the LoRA branch 2 times, most recently from 10aae55 to aaebf77 Compare March 19, 2024 07:14

DonghakPark approved these changes Mar 19, 2024

View reviewed changes

taos-ci approved these changes Mar 19, 2024

View reviewed changes

EunjuYang force-pushed the LoRA branch from aaebf77 to 41fa715 Compare March 20, 2024 09:47

taos-ci approved these changes Mar 20, 2024

View reviewed changes

EunjuYang requested a review from jijoongmoon March 21, 2024 06:33

SeoHyungjun approved these changes Apr 4, 2024

View reviewed changes

jijoongmoon approved these changes Apr 4, 2024

View reviewed changes

jijoongmoon merged commit b581db6 into nnstreamer:main Apr 4, 2024
31 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LoRA] feat/ add LoRA to fc_layer @open sesame 03/07 10:42 #2442

[LoRA] feat/ add LoRA to fc_layer @open sesame 03/07 10:42 #2442

EunjuYang commented Jan 31, 2024 •

edited

Loading

taos-ci commented Jan 31, 2024

taos-ci left a comment

lhs8928 Feb 1, 2024

lhs8928 left a comment

taos-ci left a comment

baek2sm left a comment

taos-ci commented Mar 6, 2024

taos-ci left a comment

taos-ci commented Mar 7, 2024

taos-ci left a comment

taos-ci left a comment

taos-ci left a comment

taos-ci left a comment

EunjuYang commented Mar 18, 2024

djeong20 Mar 19, 2024

EunjuYang Mar 20, 2024

DonghakPark left a comment

taos-ci left a comment

taos-ci left a comment

SeoHyungjun left a comment

jijoongmoon left a comment

                 }
               }
+              /**

[LoRA] feat/ add LoRA to fc_layer @open sesame 03/07 10:42 #2442

[LoRA] feat/ add LoRA to fc_layer @open sesame 03/07 10:42 #2442

Conversation

EunjuYang commented Jan 31, 2024 • edited Loading

taos-ci commented Jan 31, 2024

taos-ci left a comment

Choose a reason for hiding this comment

lhs8928 Feb 1, 2024

Choose a reason for hiding this comment

lhs8928 left a comment

Choose a reason for hiding this comment

taos-ci left a comment

Choose a reason for hiding this comment

baek2sm left a comment

Choose a reason for hiding this comment

taos-ci commented Mar 6, 2024

taos-ci left a comment

Choose a reason for hiding this comment

taos-ci commented Mar 7, 2024

taos-ci left a comment

Choose a reason for hiding this comment

taos-ci left a comment

Choose a reason for hiding this comment

taos-ci left a comment

Choose a reason for hiding this comment

taos-ci left a comment

Choose a reason for hiding this comment

EunjuYang commented Mar 18, 2024

djeong20 Mar 19, 2024

Choose a reason for hiding this comment

EunjuYang Mar 20, 2024

Choose a reason for hiding this comment

DonghakPark left a comment

Choose a reason for hiding this comment

taos-ci left a comment

Choose a reason for hiding this comment

taos-ci left a comment

Choose a reason for hiding this comment

SeoHyungjun left a comment

Choose a reason for hiding this comment

jijoongmoon left a comment

Choose a reason for hiding this comment

EunjuYang commented Jan 31, 2024 •

edited

Loading