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

[ App ] Update Multi_Input example #2743

Merged
merged 2 commits into from
Oct 4, 2024

Conversation

EunjuYang
Copy link
Contributor

  • This PR contains two commits:
    • c37a14e: Updating the multi-input example
      • In the previous version, the example builds a graph having multiple inputs with the same shape, which couldn't detect the bug (input-order-mismatch issue in multi-input layer. #2660).
      • To provide proper example to handle multiple inputs, it is desired to create a model taking multiple inputs with different shapes. To this end, the main application is updated.
      • If we just update the example to have different inputs, one can face the error by the issue; this commit shows how to handle the issue by updating input feeding in reverse way.
      • It is not desirable way to handle the bug; it is required to be updated later.
    • 805b4a6: Updating the model summary print of the multi-input layer
      • Here're the example of the update

[AS-IS]

================================================================================
         Layer name          Layer type    Output dimension         Input layer
================================================================================
             input2               input             1:1:8:2
--------------------------------------------------------------------------------
             input1               input             1:1:4:2
--------------------------------------------------------------------------------
             input0               input             1:1:2:2
--------------------------------------------------------------------------------
            concat0              concat            1:1:14:2              input0
                                                    1:1:4:2              input1
                                                    1:1:8:2              input2
--------------------------------------------------------------------------------
   fully_connected0     fully_connected            1:1:14:5             concat0
--------------------------------------------------------------------------------
fully_connected0/ac          activation            1:1:14:5    fully_connected0
--------------------------------------------------------------------------------
               mse0                 mse            1:1:14:5 fully_connected0/ac
================================================================================

[TO-BE]

================================================================================
          Layer name          Layer type    Output dimension         Input layer
================================================================================
              input2               input             1:1:8:2
--------------------------------------------------------------------------------
              input1               input             1:1:4:2
--------------------------------------------------------------------------------
              input0               input             1:1:2:2
--------------------------------------------------------------------------------
             concat0              concat            1:1:14:2              input0
                                                                          input1
                                                                          input2
--------------------------------------------------------------------------------
    fully_connected0     fully_connected            1:1:14:5             concat0
--------------------------------------------------------------------------------
 fully_connected0/ac          activation            1:1:14:5    fully_connected0
--------------------------------------------------------------------------------
                mse0                 mse            1:1:14:5 fully_connected0/ac
================================================================================

- This commit is related to issue nnstreamer#2660
- When using multi-inputs, users must feed the data in reverse order due
to a known bug that needs fixing. In the current version, the input must
be provided in reverse order, which was not shown in the previous
example where random data with the same dimensions were used.
- To provide a more accurate example to NNTrainer users, I have
temporarily updated this example.
- Once the issue is handled, further updates will be necessary.

Signed-off-by: Eunju Yang <[email protected]>
@taos-ci
Copy link

taos-ci commented Oct 2, 2024

📝 TAOS-CI Version: 1.5.20200925. Thank you for submitting PR #2743. 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
Copy link

taos-ci commented Oct 2, 2024

To contributor, We have used 'Signed-off-by:' notation by default to handle the license issues, that result from contributors. Note that 'Is there a Signed-off-by line?' is important because lawyers tell us we must have to it to cleanly maintain the open-source license issues even though it has nothing to do with the code itself.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

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

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

@taos-ci
Copy link

taos-ci commented Oct 2, 2024

To contributor, We have used 'Signed-off-by:' notation by default to handle the license issues, that result from contributors. Note that 'Is there a Signed-off-by line?' is important because lawyers tell us we must have to it to cleanly maintain the open-source license issues even though it has nothing to do with the code itself.

Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

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

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

@DonghakPark
Copy link
Member

Here is CI error log

             #### signed-off-by check result ####            
 * ../report/0001-Print-Update-print-result-of-model-summary.patch 
============== commit body: start =====================
[DEBUG] 
[DEBUG] - This commit updates the model summary print of the layer with multiple
[DEBUG] inputs.
[DEBUG] [ASIS]
============== commit body: end   =====================

maybe TAOS bug Recognizing "-------------------" as the end of the message

Copy link
Member

@DonghakPark DonghakPark left a comment

Choose a reason for hiding this comment

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

Just Question) I am currently working on an app that also has a multi-input scenario, have you ever compared its results with those of other frameworks? like Torch, Tensorflow?


```

- **[Note]** Users should feed the multi-input in reverse order because the model is structured in a reversed manner internally. This is a known issue for us, and we plan to address it soon.
Copy link
Member

Choose a reason for hiding this comment

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

I think it's great that it is illustrated in such a way that developers can easily understand it.

@EunjuYang
Copy link
Contributor Author

Just Question) I am currently working on an app that also has a multi-input scenario, have you ever compared its results with those of other frameworks? like Torch, Tensorflow?

For this example app, no. But I worked on another model, named AuXFT, which takes multiple inputs; I compared the pytorch output for that example. For the case, the multi-input itself worked well.

- This commit updates the model summary print of the layer with multiple
inputs.

[ASIS]
             concat0              concat            1:1:14:2              input0
                                                     1:1:4:2              input1
                                                     1:1:8:2              input2

[TOBE]
             concat0              concat            1:1:14:2              input0
                                                                          input1
                                                                          input2

Signed-off-by: Eunju Yang <[email protected]>
Copy link

@taos-ci taos-ci left a comment

Choose a reason for hiding this comment

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

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

Copy link
Contributor

@baek2sm baek2sm left a comment

Choose a reason for hiding this comment

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

It seems to be a good improvement. If the topological sort of the current implementation is always reversed in the order of the input layer, why don't we modify the implementation of the topological sort to arrange it to the regular arrangement? It seems that it can be used more intuitively without affecting performance.

Anyway, this PR seems to be a great example for users. LGTM!

@jijoongmoon jijoongmoon merged commit f222ecf into nnstreamer:main Oct 4, 2024
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants