Exit early from candidates without passing propperties #212
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.
Exit early if candidates do not pass.
Intro
In this PR, we fix an issue relating to evaluating regions in the
analyze.merge_candidate_regions.Merger
and (especially)analyze.merge_candidate_regions.MergerCached
. In these methods, a list of regions is evaluated if:To achieve this, we first sort all available regions based on if they pass the predefined criterion, and then the size of the region. Then we can iteratively start performing the second step (expanding the region).
Issue
One advised method (also see the example notebook) is to add a set of small regions that could overlap/be adjacent to a passing region to get a large region. However, to save computational expense, we often cache the results of these small regions as nan-values (such that they don't pass the predefined criterion). This is done in
MergerCached.set_all_caches_as_false
. The issue resides in these regions. Since while we indeed correctly sort the available regions and iteratively resolve them, we did not check if the remaining candidates have would pass the predefined criterion.This resulted in the small regions also being evaluated (if they would be merge-able during step 2) without explicitly checking step 1.
Solution
This PR adds a check that step 1 is performed every time we consider a new candidate before we continue with step 2.
Effect
This is particularly relevant when adding regions and using
MergerCached.set_all_caches_as_false
. For theMerger
-class the effect is minimal (when adding small regions there is no method to tell theMerger
method to consider the regions any differently from other regions).