Fixes and improvements for FPCS and KFPCS #6073
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
mvieth
added
module: registration
changelog: fix
mvieth
force-pushed
the
fpcs_fixes
branch
6 times, most recently
from
6e047ec
to
cca176a
Compare
- Replace bad random sampling: previously, the random sampling was not guaranteed to give the user-specified number of samples, was not guaranteed to choose all points with the same probability, and might choose a point more than once. The new approach works similar to the RandomSample class - Number of threads can now be set to 0 to use automatic setting - linkMatchWithBase is not used any more. It has a bug that some indices are duplicated and others dropped. It is not necessary to use it any way because match and base_indices are already correctly ordered - ids and dists_sqr are now created with the correct size (no resize necessary later) - Add debug prints, improve documentation - fpcs and kfpcs tests: set fine tuned score threshold, early termination if a good solution is found. - kfpcs test: set higher maximum number of iterations (was previously automatically estimated as 19). This should fix the random (rare) failures on Azure pipelines
|
Looks good, but just got two questions: Why not use the RandomSample class to get samples? Should linkMatchWithBase be removed?, since its only used in those two classes, though someone could have inherited from FPCSInitialAlignment we need to deprecate it first? |
larshg
approved these changes
Jul 4, 2024
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.
The new approach works similar to the RandomSample classNow the RandomSample class is used