Merged
Conversation
ktsiknos-brainchip
requested changes
Dec 5, 2025
Collaborator
ktsiknos-brainchip
left a comment
There was a problem hiding this comment.
Please reorganize you work for a better reviewer experience:
- follow https://www.conventionalcommits.org/en/v1.0.0/ for commit messages formatting
- squash your work to present simple updates and their final form to reviewers. I would expect no more that 1 or 2 commits here (e.g introducing post_filer.py then update check_model.py and sanity-check.yml, and anther for https://github.com/Brainchip-Inc/models/pull/21/files#r2592004677)
97bee9c to
3b7fbd6
Compare
2d32b03 to
1301d18
Compare
Comment on lines
+37
to
+45
| if os.path.exists(model_file): | ||
| # Move this step of the YAML workflow into the Python script | ||
| # Download the file via Git LFS if necessary | ||
| # git lfs pull --include="$f" --exclude="" | ||
| subprocess.run(["git", "lfs", "pull", | ||
| "--include", model_file,"--exclude", ""], | ||
| capture_output=True) | ||
|
|
||
| process_model(model_file) |
Contributor
There was a problem hiding this comment.
I believe you are able to simplify this as :
Suggested change
| if os.path.exists(model_file): | |
| # Move this step of the YAML workflow into the Python script | |
| # Download the file via Git LFS if necessary | |
| # git lfs pull --include="$f" --exclude="" | |
| subprocess.run(["git", "lfs", "pull", | |
| "--include", model_file,"--exclude", ""], | |
| capture_output=True) | |
| process_model(model_file) | |
| process_model(model_file) |
To avoid to change the previous script approach
.github/workflows/sanity-check.yml
Outdated
| done <<< "${{ steps.find_files.outputs.files }}" | ||
|
|
||
| FILES="${{ steps.find_files.outputs.files }}" | ||
| python CI/post_filter.py -- CI/check_model.py --models $FILES |
Contributor
There was a problem hiding this comment.
Please add git ls before to run the script
git lfs pull --include $FILES --exclude ""
Contributor
There was a problem hiding this comment.
Or maybe this could be work: git lfs fsck --pointers BASE..HEAD
fernez-brainchip
previously approved these changes
Dec 8, 2025
nguilbaud-brainchip
previously approved these changes
Dec 8, 2025
abih-brainchip
previously approved these changes
Dec 8, 2025
3d821d9
ce13dc1 to
76a7ea4
Compare
Contributor
Author
|
All change requests have been processed. Maintenance and development: If new messages appear in new versions of TensorFlow, CUDA, CUDD, etc., please extend the list of regular expressions for |
ktsiknos-brainchip
approved these changes
Dec 9, 2025
jmejia-brainchip
approved these changes
Dec 9, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After numerous attempts, we obtained a fully filtered stream without adding too much complexity to the GitHub Action script.
The filtered buffers are displayed at the end of the process associated with the action: "Run model checks".
I suggest we validate this work.