Fix singularity alignment pair search#11
Open
mheistermann wants to merge 1 commit intonicopietroni:masterfrom
Open
Fix singularity alignment pair search#11mheistermann wants to merge 1 commit intonicopietroni:masterfrom
mheistermann wants to merge 1 commit intonicopietroni:masterfrom
Conversation
Only use pairs where all sides on the path consist of only a single subside, as described in the paper.
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
1 participant
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 noticed a difference between code and paper:
It will add alignment constraints for pairs of charts where some sides on the path between them have more than one subside. I assume this is unintentional (as the induced alignment constraints will in general not correspond to proper alignment)?.
This patch adjusts
qr_ilp.cppto only use pairs where all sides on the path consist of only a single subside.I also noticed the code explicitly disables constraints for self-pairings (comparing
currentChartIdvscId) - is this intentional?Fair warning: I did not check how this affects the quality of results.
Hope you'll find this PR useful!
(By the way, thank you very much for sharing your code and thus enabling people to experiment with it and build on top of it!)