Skip to content

Conversation

@yoursanonymous
Copy link

##Summary
This PR addresses TODOs in the codebase by implementing proper memory cleanup for the neural network component and documenting magic numbers used for buffer padding.

- Implemented th_nn_deinit() to properly free neural network memory

- Added documentation for magic number padding in ee_kws.c and ee_abf_f32.c

- Resolved TODO comments about memory cleanup and padding justification
@joseph-yiu
Copy link
Contributor

Please separate this into multiple patches. The NN cleanup need to be linked to #81. But I can't do that when your PR is combining multiple changes. Please separate the patches into 3 pull requests:

(1) The new file .github/workflow/build.yml duplicated with #79
(2) The NN cleanup
(3) Documentation improvement
In future, please also provide the GitHub issue that you are trying to address in the PR description. It is okay here now as there are only a few issues and PRs. But in bigger projects it can be difficult to see the connections.
Thanks and regards,
Joseph

@yoursanonymous yoursanonymous changed the title Add Neural Network Cleanup and Document Magic Numbers Add Neural Network Cleanup Feb 2, 2026
@yoursanonymous
Copy link
Author

Thanks a lot for the detailed feedback, Joseph really appreciate it.

I will split this PR into multiple pull requests as suggested:

-A PR for the NN cleanup (linked to #84)

-A separate PR for the documentation improvements

-I will drop the duplicated CI workflow change since it’s already covered in #79

Thanks again for pointing this out and for the guidance.

@joseph-yiu
Copy link
Contributor

Another suggestion, if you don't mind:

Instead of calling the new function th_nn_deinit(), please use the name th_cleanup().

  • "Cleanup" is clearer than "deinit", which isn't a proper word.
  • The function might not be exclusively use for NN related cleanup. In some devices it could include cleanup for DSP hardware accelerator (if that exists).

regards,
Joseph

@yoursanonymous
Copy link
Author

Thanks for the suggestion, Joseph

I have renamed th_nn_deinit() to th_cleanup() and updated all call sites accordingly. The new name is clearer and leaves room for future cleanup beyond NN-specific resources.

Best regards,
Vinayak

joseph-yiu added a commit that referenced this pull request Feb 10, 2026
@joseph-yiu
Copy link
Contributor

Merged to dev_2026q1 branch.
regards,
Joseph

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.

2 participants