Lineage Registration: dynamic spacial registration #28
Conversation
For now the SelectedProject class simply wraps around the WindowManager and project name. But it allows to store addition options like the first timepoint to consider etc.
The lineage registration dialog already has two text fields to enter the first time points to be used for registration. The LineageRegistrationAlgorithm is now changed to only register spots after that time point.
…mation The algorithm previously used the first spot of each root cell to estimate a coordinate transform between the two embryos. But we noticed for example for Macrostomum that the cells move a lot before cell division So which cell location (spot) along the trajectory of a cell would be used for estimating the coordinate transform depends on the time at which the recording starts. So it was basically random. Now the algorithm uses the position of the dividing spot, which is better defined.
…arch The code is simpler and DepthFirstIterator is faster the DepthFirstSearch.
…algorithms There are different ideas to improve the spacial registration of two embryos during lineage registration. This commit introduces a interface SpacialRegistration that allows to plug in these improved spacial registration algorithms.
…tion The LineageRegistrationFrame new has dynamic spacial registration based on roots selected as standard.
1. select the newly added spots and links 2. set and undo point 3. acquire the write lock on the graph before changing it.
There was a problem hiding this comment.
This PR modifies the method how the lineage registration is done and adds new methods.
-
It is important to mention that this PR is not 100% in line with the issue description (Lineage Registration: improve registration of all timepoints #22). The "fixed" option is modified. It seems to improve the registration, but is not working exactly as before, as the issue description claims, but this can be solved by providing some documentation about the three registration methods.
-
It is hard to judge, if the dynamic registration really improves the registration. It seems so, but one can also find examples, where it worsens the registration.
-
Since the "Color paired lineages" function is now the only function that is independent from the chosen registration, it may be moved to different dialog. Users otherwise may wonder why this function always leads to the same result, no matter, which registration method is active.
-
In TrackScheme, there is a bug, when adding a new center Spot. TrackScheme needs to be refreshed with
Ctrl+Delafter a new center spot is added, before the user can see it in TrackScheme. -
Regarding registration with the "landmarks" tag set: the documentation should inform users that in case of multiple "landmarks" tag set, the first one is chosen by the algorithm.
-
Please consider renaming "spacial" to "spatial". Both versions of the word exist, but the version with 't' seems to be the more common one
The newly added spots where not shown in the TrackScheme. This was because the plugin didn't call graph.notifyGraphChanged. This problem is now fixed. Additionally the order of actions done by the plugin was changed to be the same EditBehaviors.AddSpotBeghavior. The following order is now used: 1. write lock graph 2. add spots 3. notify graph changed 4. set undo redo point 5. select new spots 6. unlock graph
Don't load the gui state. But open a "TrackScheme Branch" and main window (with nice a window titles) for both projects.
The are simple reasons that then lineage registration fails: - the datasets have less than 3 dividing cell lineages - both dataset start at a different stage - cells are not properly labeld - "landmarks" tag set has too few tags This commit improves the exception messages that are printed in those cases.
|
@stefanhahmann Thank you for testing the code and sharing your opinions. I agree with your concerns. Some problems are fixed now. Here are my replies to the individual points. I think the PR is good enough to be merged now.
That's true. There is a small different between the old behavior and the "fixed spacial registration" option. The old behavior used spot coordinates on timepoint 0 for registration. The new behavior uses coordinates of the root cells just before cell division.
I agree that it is hard to judge. But there are reasons to believe that the dynamic registration works better:
I agree with your concerns. But a separate dialog, where you select dataset and timepoints, is not worth the struggle. Lets instead write documentation that makes things clear.
Fixed in #e1dce454c1cf3eba6ebfb4888dbb38f4cad6b4ef
I agree.
done |
There was a problem hiding this comment.
There is a comment in a related PR that does not seem to have been addressed:
#27 (comment)
The CI seems to fail on DynamicLandmarkRegistrationTest.testLineageRegistrationAlgorithm()
It is not clear to me, if this is related to this PR or perhaps to mastodon-sc/mastodon#225
Could you please check?
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RefCollectionUtils.java
Outdated
Show resolved
Hide resolved
…aphs This change simplifies many methods in the LineageRegistration class since the modelA and modelB parameters are no longer necessary as they are contained in the RegisteredGraph object.
Closes #22