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

mlearning/tflite-micro: add a config option to redirect micro log to syslog #2882

Merged
merged 1 commit into from
Dec 10, 2024

Conversation

anchao
Copy link
Contributor

@anchao anchao commented Dec 9, 2024

Summary

mlearning/tflite-micro: add a config option to redirect micro log to syslog

new config option TFLITEMICRO_SYSLOG to redirect micro log to syslog

Signed-off-by: chao an anchao@lixiang.com

Impact

N/A

Testing

sim/tflm

$ cmake -B build -DBOARD_CONFIG=sim/tflm -GNinja
nsh> tflm_hello
[    3.720000] [ 4] 0 (id=0): size=16, offset=0, first_used=0 last_used=1
[    3.720000] [ 4] 1 (id=1): size=64, offset=64, first_used=1 last_used=2
[    3.720000] [ 4] 2 (id=2): size=64, offset=0, first_used=2 last_used=3
[    3.720000] [ 4] 3 (id=3): size=16, offset=64, first_used=3 last_used=3
[    3.720000] [ 4]  0: 0000000000...................................................................... (1k)
[    3.720000] [ 4]  1: 0000000000..............................1111111111111111111111111111111111111111 (1k)
[    3.720000] [ 4]  2: 22222222222222222222222222222222222222221111111111111111111111111111111111111111 (1k)
[    3.720000] [ 4]  3: 22222222222222222222222222222222222222223333333333.............................. (1k)
[    3.720000] [ 4] 
[    3.720000] [ 4] "Unique Tag","Total ticks across all events with that tag."
[    3.720000] [ 4] FULLY_CONNECTED, 0
[    3.720000] [ 4] "total number of ticks", 0
[    3.720000] [ 4] 
[    3.720000] [ 4] [RecordingMicroAllocator] Arena allocation total 2344 bytes
[    3.720000] [ 4] [RecordingMicroAllocator] Arena allocation head 128 bytes
[    3.720000] [ 4] [RecordingMicroAllocator] Arena allocation tail 2216 bytes
[    3.720000] [ 4] [RecordingMicroAllocator] 'TfLiteEvalTensor data' used 240 bytes with alignment overhead (requested 240 bytes for 10 allocations)
[    3.720000] [ 4] [RecordingMicroAllocator] 'Persistent TfLiteTensor data' used 128 bytes with alignment overhead (requested 128 bytes for 2 tensors)
[    3.720000] [ 4] [RecordingMicroAllocator] 'Persistent buffer data' used 1152 bytes with alignment overhead (requested 1100 bytes for 7 allocations)
[    3.720000] [ 4] [RecordingMicroAllocator] 'NodeAndRegistration struct' used 192 bytes with alignment overhead (requested 192 bytes for 3 NodeAndRegistration structs)
[    3.720000] [ 4] 0 (id=0): size=16, offset=0, first_used=0 last_used=1
[    3.720000] [ 4] 1 (id=1): size=64, offset=64, first_used=1 last_used=2
[    3.720000] [ 4] 2 (id=2): size=64, offset=0, first_used=2 last_used=3
[    3.720000] [ 4] 3 (id=3): size=16, offset=64, first_used=3 last_used=3
[    3.720000] [ 4]  0: 0000000000...................................................................... (1k)
[    3.720000] [ 4]  1: 0000000000..............................1111111111111111111111111111111111111111 (1k)
[    3.720000] [ 4]  2: 22222222222222222222222222222222222222221111111111111111111111111111111111111111 (1k)
[    3.720000] [ 4]  3: 22222222222222222222222222222222222222223333333333.............................. (1k)
[    3.720000] [ 4] 0 (id=0): size=16, offset=16, first_used=0 last_used=1
[    3.720000] [ 4] 1 (id=1): size=16, offset=0, first_used=1 last_used=2
[    3.720000] [ 4] 2 (id=2): size=16, offset=16, first_used=2 last_used=3
[    3.720000] [ 4] 3 (id=3): size=16, offset=0, first_used=3 last_used=3
[    3.720000] [ 4]  0: ................0000000000000000................................................ (1k)
[    3.720000] [ 4]  1: 11111111111111110000000000000000................................................ (1k)
[    3.720000] [ 4]  2: 11111111111111112222222222222222................................................ (1k)
[    3.720000] [ 4]  3: 33333333333333332222222222222222................................................ (1k)
[    3.720000] [ 4] ~~~ALL TESTS PASSED~~~
nsh> poweroff

@nuttxpr
Copy link

nuttxpr commented Dec 9, 2024

[Experimental Bot, please feedback here]

No, this PR does not fully meet the NuttX requirements.

  • Summary: While it describes the change, it lacks crucial information like why this change is necessary. Is there a problem with the current logging method? Does this improve performance, debugging, or user experience?
  • Impact: "N/A" is insufficient. While the impact might be minimal, you must explicitly state that and justify it. For instance, does this change the memory usage? Does it add any new dependencies? Consider each point specifically. If the answer is truly "NO" for each, then state "NO" and briefly explain why (e.g., "Impact on build: NO, only adds a config option").
  • Testing: The testing logs don't show a comparison. You need logs before the change using the old logging method, and logs after the change demonstrating the redirection to syslog. Simply providing the steps to build and run the test isn't enough. Show the actual output difference. Also, specify the details of the sim/tflm target. What version of NuttX are you using? What specific simulator is this (QEMU?)?

The author needs to revise the PR to address these shortcomings before it can be properly reviewed.

Copy link

@cederom cederom 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 @anchao :-) Would this also work with Make builds? I can see only CMake updates? :-)

…syslog

new config option TFLITEMICRO_SYSLOG to redirect micro log to syslog

Signed-off-by: chao an <anchao@lixiang.com>
@anchao
Copy link
Contributor Author

anchao commented Dec 10, 2024

Thank you @anchao :-) Would this also work with Make builds? I can see only CMake updates? :-)

@cederom thanks for your reminder. I also updated the makefile. Please review again.

@xiaoxiang781216 xiaoxiang781216 merged commit 9c5a2ad into apache:master Dec 10, 2024
25 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.

4 participants