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
Tree-based agglomeration #32
Merged
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
Don't merge this! Turns out Travis was "passing" despite failing tests! (See #33.) Will rebase and fix failing tests. |
MergeQueue.peek() was designed to look at the next item in the merge queue without popping it. However, the next item *could* be invalid, and cause a merge of a valid item past threshold! This commit fixes that by having peek() discard invalid items until a valid one is at the front of the queue.
Tree merging was causing the sp2segment map to be incorrect when the merge boundaries were being refined. Postponing the merge until after boundary refinement fixes the issue.
The region intersection graph allows quick computation of current VI and rand index during an agglomeration run.
I doubled the size of the contingency table, but when there is no contingency table, I hadn't doubled the size of the ones() array, resulting in an IndexError. This is now fixed.
- added optimized.pyx for Cython, and tests for its functions - added missing kwarg to classify.py function - modified imio.read_h5_stack to not fail with kwargs for other imio function args - stop agglo.py tracking 'extent' set after features are built - optimize 'histogram' and 'moments' feature computations with Cython - added 'contact' feature, which helped win SNEMI3D - added adjusted Rand error metric to evaluate.py - dramatically improved testing framework - updated travis build to use Conda - updated README with all the goodies - other minor improvements
Changes Unknown when pulling 740bebb on jni:tree-agglo into * on janelia-flyem:master*. |
I'm merging this as a precursor to a release. Boom! |
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.
I've been thinking about optimizing gala. I think one of the first steps is to stop merging segments during agglomeration, and just keep track of the merges in a tree. I've created a Python tree library viridis (after Morelia viridis, the green tree python) and use it to keep track of the merge tree. All the segment merging logic is gone, simplifying the codebase while increasing efficiency. It passes all the current tests (admittedly few, but they cover the essentials of agglomeration and feature computation).
Please let me know if there are any objections to this change.