Skip to content

feat(deploy_to_autoware): rework metadata script#36

Merged
amadeuszsz merged 1 commit intotier4:mainfrom
amadeuszsz:feat/rework-model-meta-script
May 6, 2025
Merged

feat(deploy_to_autoware): rework metadata script#36
amadeuszsz merged 1 commit intotier4:mainfrom
amadeuszsz:feat/rework-model-meta-script

Conversation

@amadeuszsz
Copy link
Collaborator

Summary

An update for metadata info script.

Change point

Latest PR just added script without deeper investigation. This PR refines meta information settings in order to achieve an appropriate logging during Autoware runtime.

Note

Domain field refers to ONNX operator set, not task type. So far we use default operator set, therefore ai.onnx is the only choice (see https://github.com/onnx/onnx/blob/main/docs/Versioning.md#released-versions).

Test performed

  • Log
Log output

Signed-off-by: Amadeusz Szymko <amadeusz.szymko.2@tier4.jp>
@amadeuszsz amadeuszsz self-assigned this May 1, 2025
@amadeuszsz amadeuszsz requested a review from scepter914 May 1, 2025 08:34
Copy link
Contributor

@scepter914 scepter914 left a comment

Choose a reason for hiding this comment

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

Thank you for contribution 👍

I'm planning to set up a pipeline for deployment from now on, so I intend to use this script. Really appreciate it.

@amadeuszsz
Copy link
Collaborator Author

@scepter914
Thanks.
I figured out we use custom operators in BEVFusion and PTv3. I will reconsider the best way of developing custom ops for AWML and address it in separate PR.

@amadeuszsz amadeuszsz merged commit bf29e40 into tier4:main May 6, 2025
2 checks passed
@amadeuszsz amadeuszsz deleted the feat/rework-model-meta-script branch May 6, 2025 23:38
@KSeangTan
Copy link
Collaborator

KSeangTan commented May 7, 2025

Hi @amadeuszsz

Just a question, is it better if we make the code more object-oriented, and each project only uses the class in their own deployment script? In this way, we can set up metadata for different projects

@scepter914
Copy link
Contributor

scepter914 commented May 7, 2025

@KSeangTan
Method 1 (current approach):

  1. A script that generates the ONNX model
  2. A script that writes the meta version

Method 2 (the approach proposed by Kok-san):
Create a class (API) for writing the meta version, and have each project import and use that API when generating the ONNX model to write the metadata.

Is that correct?

Technically, either method is feasible.
However, from a software design perspective, I feel that separating concerns into standalone CLI tools (as in Method 1) results in more loosely coupled architecture.

For example, with Method 2, if the class is modified, all the ONNX generation scripts in each project would also need to be updated.
In contrast, with Method 1, even if the deploy script is changed, the project code itself can remain untouched.
The command example in the README would need to be updated.

Of course, if a project requires embedding some special metadata during deployment, it’s certainly possible to prepare a project-specific script.
But I believe that’s the exception rather than the rule and it is additional engineering from a common solution.
So I think it is better that a simple standalone script per project might result in better maintainability overall.

@amadeuszsz
Copy link
Collaborator Author

@KSeangTan

Just a question, is it better if we make the code more object-oriented, and each project only uses the class in their own deployment script? In this way, we can set up metadata for different projects

I like this idea! If we have much more shared features, we could go for a design of AWModel meta class and make it standard for all AWML projects.

@KSeangTan
Copy link
Collaborator

KSeangTan commented May 8, 2025

Hi @scepter914 @amadeuszsz

Yes, you are right that the method 1 is better in maintainability, and it also looks good to me.

I suppose we can think about the shared metadata design that will not be changed for a long time, and every project uses that AWMLModelMeta as mentioned by @amadeuszsz

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants