Taxonomy rollup cutoff with a threshold #37
Merged
Conversation
This file contains 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
from 550ms per frame to 120ms per frame on my iPhone 13 pro
This enables us to set a threshold of 0 for not filtering out any nodes at all.
When setting a taxon filter and removing it the frame processor was not removing the filter internally. This should fix that.
…nodes Should improve cv speed on Android
from 550ms per frame to 120ms per frame on my iPhone 13 pro
This enables us to set a threshold of 0 for not filtering out any nodes at all.
When setting a taxon filter and removing it the frame processor was not removing the filter internally. This should fix that.
…nodes Should improve cv speed on Android
…list/vision-camera-plugin-inatvision into taxonomy-rollup-cutoff-2
jtklein
commented
Nov 1, 2024
jtklein
commented
Nov 1, 2024
jtklein
commented
Nov 1, 2024
alexshepard
reviewed
Nov 18, 2024
alexshepard
reviewed
Nov 18, 2024
ios/VisionCameraPluginInatVision/VisionCameraPluginInatVisionModule.m
Outdated
Show resolved
Hide resolved
alexshepard
requested changes
Nov 18, 2024
There was a problem hiding this comment.
I have only reviewed the objective-c code.
I would suggest two changes:
- move the cutoff property into the header file so you don't have to make a custom setter method - this is how we normally do things in Objective-C, instead of making our own setters and getters,
- add time to process CV frames to the response so that could can either log it or do something more clever on the react side of things.
jtklein
added a commit
that referenced
this pull request
Nov 20, 2024
* Update Podfile.lock * Taxonomy rollup cutoff with a threshold (#37) * apply a cutoff before taxonomy cutoff from 550ms per frame to 120ms per frame on my iPhone 13 pro * On Android Implement taxonomy rollup only for nodes with wanted scores * Expose taxonomy cutoff threshold to JS side * Destructure taxonomyRollupCutoff out of options * Update VCPTaxonomy.m * Set taxonomy rollup cutoff value from plugin * Update comment * Stub for option validation * Include node also if exactly on the threshold score This enables us to set a threshold of 0 for not filtering out any nodes at all. * Default threshold to 0 on iOS * Comments * Also attach options to the stored result * Log number of nodes that are in the final aggregated set * Default to 0.0 on Android * Inclusive check of threshold on Android * Log number of nodes in aggregate scores on Android * Remove unnecessary null checks * Fix bug with taxon filter on Android When setting a taxon filter and removing it the frame processor was not removing the filter internally. This should fix that. * Use null in example app instead of undefined as we do in Seek * If a taxon filter is applied do not aggregate scores of filtered out nodes Should improve cv speed on Android * Do not include inner nodes if their summed score is 0 * apply a cutoff before taxonomy cutoff from 550ms per frame to 120ms per frame on my iPhone 13 pro * On Android Implement taxonomy rollup only for nodes with wanted scores * Expose taxonomy cutoff threshold to JS side * Destructure taxonomyRollupCutoff out of options * Update VCPTaxonomy.m * Set taxonomy rollup cutoff value from plugin * Update comment * Stub for option validation * Include node also if exactly on the threshold score This enables us to set a threshold of 0 for not filtering out any nodes at all. * Default threshold to 0 on iOS * Comments * Also attach options to the stored result * Log number of nodes that are in the final aggregated set * Default to 0.0 on Android * Inclusive check of threshold on Android * Log number of nodes in aggregate scores on Android * Remove unnecessary null checks * Fix bug with taxon filter on Android When setting a taxon filter and removing it the frame processor was not removing the filter internally. This should fix that. * Use null in example app instead of undefined as we do in Seek * If a taxon filter is applied do not aggregate scores of filtered out nodes Should improve cv speed on Android * Do not include inner nodes if their summed score is 0 * Refactor checking of option * Fix log * Also send back full options dict for predictions from image * Fix comment * Remove assert because validation happens on JS side * Only add scores higher 0 also on iOS * Move property taxonomyRollupCutoff to header file * Return timeElapsed to caller * Add timeElapsed to all possible cv results * Add timeElapsed to result interface --------- Co-authored-by: Alex Shepard <[email protected]> --------- Co-authored-by: Alex Shepard <[email protected]>
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.
This adds a customizable threshold to the computer vision workflow during frame processing.
During the recursive aggregation of scores for the inner taxonomic nodes a threshold is applied and every node with a lower computer vision score is excluded from the set of nodes on which to find the best predictions from.
Technically there is a breaking change in the structure of the returned dictionary for predictions from file but neither Seek nor iNat has destructured the
uriparam, so I will not bump major version.@alexshepard can you please double check I took the rollup changes on iOS correctly, and how to set the threshold from outside the VCPTaxonomy class.
P.S: I realized that main is broken atm, so I merge this into a previous stable commit.