[tools] introduce circle plus generator #12977

zetwhite · 2024-05-09T13:00:31Z

This introduces a circle plus generator, which helps handle a circle file with training hyperparameters.
As a first feature, it checks whether a circle file contains training parameters or not.

ONE-DCO-1.0-Signed-off-by: seunghui youn sseung.youn@samsung.com

draft : [draft] add circle_plus_gen as tool #12975
related issue : [onert/train] Let's introduce circle+ generating tools #12650

zetwhite · 2024-05-09T14:03:27Z

tools/circle_plus_gen/README.md

+
+## Inject training parameters using json file
+
+<!--to be updated -->


I'll add this feature in next PR

tools/circle_plus_gen/lib/train_info.py

This introduces a circle plus generator, which helps handle a circle file with training hyperparameters. As a first feature, it checks whether a circle file contains training parameters or not. ONE-DCO-1.0-Signed-off-by: seunghui youn <sseung.youn@samsung.com>

hseok-oh · 2024-05-10T01:50:20Z

@zetwhite Could you reduce example model file size? (under 5KB if possible)

BTW, I failed to open example model on circledump and netron.

 ./build/out/bin/circledump tools/circle_plus_gen/example/mnist.circle 
Dump: tools/circle_plus_gen/example/mnist.circle

Segmentation fault (core dumped)

tools/circle_plus_gen/README.md

jyoungyun · 2024-05-13T00:56:04Z

Good~ This PR looks good to me :)

The attached mnist_with_tparam.circle file contains CTR1 for TRAINING_FILE_IDENTIFIER. Please use this PR to regenerate the circle file.

(added)
To recognize the mnist_with_tparam.circle file in the circle format, the CIR0 file identifier must be added when exporting the new circle file.

And the mnist.circle seems to be corrupt. Please check this file too.

details

mnist_with_tparam.circle file

00000000: 1800 0000 0000 1200 1800 0000 1400 1000  ................
...
00000050: 4349 5243 4c45 5f54 5241 494e 494e 4700  CIRCLE_TRAINING.
00000060: 0b00 0000 485b 0300 405b 0300 304b 0000  ....H[..@[..0K..
...
000000a0: 4354 5231 0000 1600 2000 1c00 0000 1b00  CTR1.... .......

correct file

00000000: 1c00 0000 4349 5230 0000 1200 1800 0000  ....CIR0........
...
00000050: 0f00 0000 4349 5243 4c45 5f54 5241 494e  ....CIRCLE_TRAIN
00000060: 494e 4700 2200 0000 6439 0000 5c39 0000  ING."...d9..\9..
...
000000f0: 76c6 ffff 0400 0000 9400 0000 2400 0000  v...........$...
00000100: 4354 5230 0000 1a00 2400 2000 1f00 1e00  CTR0....$. .....

tools/circle_plus_gen/README.md

tools/circle_plus_gen/lib/circle_plus.py

tools/circle_plus_gen/main.py

tools/circle_plus_gen/lib/train_info.py

tools/circle_plus_gen/README.md

zetwhite · 2024-05-13T08:22:13Z

I'm still updating this PR. I'll re-request the review explicitly if it is ready.

zetwhite · 2024-05-13T11:43:05Z

@zetwhite Could you reduce example model file size? (under 5KB if possible)

BTW, I failed to open example model on circledump and netron.

@hseok-oh I replace circle files with smaller-sized ones. Now it's about 1KiB.
I also checked it opened well in my local env, Could you try again?

zetwhite · 2024-05-13T11:46:28Z

@jyoungyun @ragmani I updated PR based on code reviews. Could you take another look?

@mhs4670go Thank you a lot for the detailed reviews! Thanks to your review(especially about SRP rules), I could get a much better code. Pleas take another look.

jyoungyun · 2024-05-14T00:17:15Z

tools/circle_plus_gen/README.md

+
+2. Install required pakcages. 
+
+    Currently, only `flatbuffers==24.3.25` is needed.


Suggested change

Currently, only `flatbuffers==24.3.25` is needed.

Currently, only `flatbuffers` is needed.

I think this tool does not require a specific flatbuffers version, doesn't it?

I think this tool does not require a specific flatbuffers version, doesn't it?

Maybe.
But I couldn't guarantee that it works well with other versions.

So, How about

removing the version in README.md (because we don't have to emphasize the version in readme)

keeping the version in requirements.txt (because specifying the working well version is necessary)

jyoungyun · 2024-05-14T00:29:53Z

The attached sample_tparam.circle still has an error when opening this file using netron. This file does not contain CIR0 to distinguish the circle file. When creating the circle file, the circle file identifier(CIR0) must be used. Please refer to the code below.

        builder.Finish(self.modelT.Pack(builder), self.CIRCLE_FILE_IDENTIFIER)
        with open(fname, 'wb') as f:
            f.write(builder.Output())

tools/circle_plus_gen/main.py

jyoungyun · 2024-05-14T00:46:49Z

tools/circle_plus_gen/main.py

+    '''
+    print(f"check hyperparameters in {in_circle_file}")
+
+    circle_model: CirclePlus = CirclePlus.from_file(in_circle_file)


Is there any reason to write the type explicitly?

No special reason.
I thought that.. writing type here helps the reader not to read the inside of CirclePlus.from_file.

tools/circle_plus_gen/schema/__init__.py

zetwhite · 2024-05-14T01:50:45Z

The attached sample_tparam.circle still has an error when opening this file using netron. This file does not contain CIR0 to distinguish the circle file. When creating the circle file, the circle file identifier(CIR0) must be used. Please refer to the code below

@jyoungyun sorry for your double-checks. There might be no problem this time. I uploaded it again and thank you for your thoughtful review. 😄

ONE_second on  0509/circlep_check [$?] …
➜ xxd tools/circle_plus_gen/example/sample_tpram.circle 
00000000: 1c00 0000 4349 5230 0000 1200 1800 0000  ....CIR0........
00000010: 1400 1000 0c00 0800 0000 0400 1200 0000  ................
00000020: 1400 0000 4000 0000 e003 0000 f403 0000  ....@...........
00000030: a805 0000 0100 0000 0c00 0000 0800 0c00  ................
00000040: 0800 0400 0800 0000 0600 0000 0400 0000  ................
00000050: 0f00 0000 4349 5243 4c45 5f54 5241 494e  ....CIRCLE_TRAIN
00000060: 494e 4700 0700 0000 9c03 0000 9403 0000  ING.............
00000070: 0401 0000 f400 0000 d400 0000 7000 0000  ............p...

ragmani

LGTM 👍

jyoungyun

@jyoungyun sorry for your double-checks. There might be no problem this time. I uploaded it again and thank you for your thoughtful review. 😄

This file works well. This code looks good to me. :)

hseok-oh

LGTM

zetwhite · 2024-05-14T04:32:33Z

@hseok-oh could you merge this one?
@mhs4670go is out of the office this week, so it might be hard to get his approval soon.

zetwhite · 2024-05-14T04:45:16Z

tools/circle_plus_gen/schema/__init__.py

+#
+# * flatc version 23.5.26 is used
+# ./flatc --python --gen-onefile --gen-object-api ../../../nnpackage/schema/circle_schema.fbs
+# ./flatc --python --gen-onefile --one-object-api ../../../runtime/libs/circle-schema/include/circle_traininfo.fbs


Suggested change

# ./flatc --python --gen-onefile --one-object-api ../../../runtime/libs/circle-schema/include/circle_traininfo.fbs

# ./flatc --python --gen-onefile --gen-object-api ../../../runtime/libs/circle-schema/include/circle_traininfo.fbs

typo again.. I'll fix it in next PR.

zetwhite force-pushed the 0509/circlep_check branch 4 times, most recently from 483bf38 to 9237f7d Compare May 9, 2024 13:53

zetwhite added the approval: 2 Require at least 2 approvals label May 9, 2024

zetwhite commented May 9, 2024

View reviewed changes

zetwhite force-pushed the 0509/circlep_check branch 2 times, most recently from 47ea7c2 to 1926ede Compare May 9, 2024 14:18

zetwhite force-pushed the 0509/circlep_check branch from 1926ede to 566d5e2 Compare May 9, 2024 14:23

jyoungyun reviewed May 13, 2024

View reviewed changes

tools/circle_plus_gen/README.md Outdated Show resolved Hide resolved

mhs4670go previously approved these changes May 13, 2024

View reviewed changes

replace to small sized circle

6c84c1f

zetwhite dismissed mhs4670go’s stale review via 6c84c1f May 13, 2024 06:00

ragmani reviewed May 13, 2024

View reviewed changes

tools/circle_plus_gen/README.md Outdated Show resolved Hide resolved

tools/circle_plus_gen/README.md Outdated Show resolved Hide resolved

zetwhite force-pushed the 0509/circlep_check branch from cfa2edd to 7ffa2ac Compare May 13, 2024 07:21

zetwhite added the PR/NO MERGE Please don't merge. I'm still working on this :) label May 13, 2024

zetwhite force-pushed the 0509/circlep_check branch 3 times, most recently from b99872e to b252485 Compare May 13, 2024 11:37

zetwhite requested review from jyoungyun, ragmani and mhs4670go May 13, 2024 11:38

zetwhite removed the PR/NO MERGE Please don't merge. I'm still working on this :) label May 13, 2024

update based on code-review

a4246b8

zetwhite force-pushed the 0509/circlep_check branch from b252485 to a4246b8 Compare May 13, 2024 11:50

jyoungyun reviewed May 14, 2024

View reviewed changes

update sample_tparam.circle

f75695a

zetwhite force-pushed the 0509/circlep_check branch from 044d78a to 611daaa Compare May 14, 2024 01:36

applied code review

58364bf

zetwhite force-pushed the 0509/circlep_check branch from 611daaa to 58364bf Compare May 14, 2024 01:43

zetwhite requested a review from jyoungyun May 14, 2024 01:48

ragmani approved these changes May 14, 2024

View reviewed changes

jyoungyun approved these changes May 14, 2024

View reviewed changes

hseok-oh approved these changes May 14, 2024

View reviewed changes

zetwhite commented May 14, 2024

View reviewed changes

hseok-oh merged commit 87506c7 into Samsung:master May 14, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tools] introduce circle plus generator #12977

[tools] introduce circle plus generator #12977

zetwhite commented May 9, 2024 •

edited

zetwhite May 9, 2024

hseok-oh commented May 10, 2024

jyoungyun commented May 13, 2024 •

edited

zetwhite commented May 13, 2024

zetwhite commented May 13, 2024

zetwhite commented May 13, 2024

jyoungyun May 14, 2024

zetwhite May 14, 2024

jyoungyun commented May 14, 2024

jyoungyun May 14, 2024

zetwhite May 14, 2024

zetwhite commented May 14, 2024

ragmani left a comment

jyoungyun left a comment •

edited

hseok-oh left a comment

zetwhite commented May 14, 2024

zetwhite May 14, 2024


		## Inject training parameters using json file

		<!--to be updated -->


		2. Install required pakcages.

		Currently, only `flatbuffers==24.3.25` is needed.

	Currently, only `flatbuffers==24.3.25` is needed.
	Currently, only `flatbuffers` is needed.

	# ./flatc --python --gen-onefile --one-object-api ../../../runtime/libs/circle-schema/include/circle_traininfo.fbs
	# ./flatc --python --gen-onefile --gen-object-api ../../../runtime/libs/circle-schema/include/circle_traininfo.fbs

[tools] introduce circle plus generator #12977

[tools] introduce circle plus generator #12977

Conversation

zetwhite commented May 9, 2024 • edited

zetwhite May 9, 2024

Choose a reason for hiding this comment

hseok-oh commented May 10, 2024

jyoungyun commented May 13, 2024 • edited

zetwhite commented May 13, 2024

zetwhite commented May 13, 2024

zetwhite commented May 13, 2024

jyoungyun May 14, 2024

Choose a reason for hiding this comment

zetwhite May 14, 2024

Choose a reason for hiding this comment

jyoungyun commented May 14, 2024

jyoungyun May 14, 2024

Choose a reason for hiding this comment

zetwhite May 14, 2024

Choose a reason for hiding this comment

zetwhite commented May 14, 2024

ragmani left a comment

Choose a reason for hiding this comment

jyoungyun left a comment • edited

Choose a reason for hiding this comment

hseok-oh left a comment

Choose a reason for hiding this comment

zetwhite commented May 14, 2024

zetwhite May 14, 2024

Choose a reason for hiding this comment

zetwhite commented May 9, 2024 •

edited

jyoungyun commented May 13, 2024 •

edited

jyoungyun left a comment •

edited