Skip to content
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

[BUG] Projecting neurons in the Landmarks Widget results in incorrect transformation #2285

Open
acardona opened this issue Jan 27, 2025 · 22 comments

Comments

@acardona
Copy link
Contributor

acardona commented Jan 27, 2025

In volume 1099, we attempted to use the 4 landmarks of the right mushroom body, which we checked are named correctly, and the 3D viewer shows them at the correct 3D location relative to traced Kenyon cells.

When we load neurons from Seymour using the latter's "Kenyon Cell right" annotation, or any other annotation, the transformed neurons are shown in the 3D viewer with an inverted anterior-posterior axis, and overly stretched long that same axis.

This image shows the incorrectly transformed Kenyon cells and the volume of the landmarks used:

Image

And this is how they should look like: in blue, the locally partially traced Kenyon cells:

Image

@acardona
Copy link
Contributor Author

Perhaps related: the "brain hemisphere right" landmark group of volume 1099 has an extra unnamed landmark that can't be deleted, throws this error:

Unsuccessful request: PointClassInstance matching query does not exist.

Click to show/hide detail (please include in bug reports)

Version: 2021.12.21.dev0+unknown
URL: /catmaid/fibsem/4/landmarks/groups/25788/locations/209195/
Method: DELETE
Details: Traceback (most recent call last):
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/usr/lib/python3.10/contextlib.py", line 79, in inner
    return func(*args, **kwds)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view
    return view_func(*args, **kwargs)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/rest_framework/views.py", line 509, in dispatch
    response = self.handle_exception(exc)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/rest_framework/views.py", line 469, in handle_exception
    self.raise_uncaught_exception(exc)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
    raise exc
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/rest_framework/views.py", line 506, in dispatch
    response = handler(request, *args, **kwargs)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/django/utils/decorators.py", line 46, in _wrapper
    return bound_method(*args, **kwargs)
  File "/opt/catmaid/fibsem/django/applications/catmaid/control/authentication.py", line 237, in inner_decorator
    return f(request, *args, **kwargs)
  File "/opt/catmaid/fibsem/django/applications/catmaid/control/landmarks.py", line 1133, in delete
    pci = PointClassInstance.objects.get(point=point,
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/django/db/models/manager.py", line 87, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/opt/catmaid/fibsem/django/env/lib/python3.10/site-packages/django/db/models/query.py", line 637, in get
    raise self.model.DoesNotExist(
catmaid.models.PointClassInstance.DoesNotExist: PointClassInstance matching query does not exist.

@acardona
Copy link
Contributor Author

I deleted the whole landmarks group and recreated it, but the transform error still continues.
The "brain hemisphere left" landmark group has several empty landmarks that can't be deleted either if you want to try to reproduce the error.

@acardona
Copy link
Contributor Author

We found that "MB Vertical / Medial Lobe Intersection" is a landmark that exists both in 1099 and in Seymour, but a break at getPointMatches function in catmaid-lib.js shows that this landmark is not being used: it fails to be added to the sharedLandmarkIds.

While on the break, I tested whether fromLandmarkNames.has("MB Vertical / Medial Lobe Intersection") and indeed prints true, so I am very confused as to why it didn't return true when stepping into the loop that compares the landmarks by name.

The end result is that it uses only 3 landmarks to define a 3D transform, and therefore it's very wrong.

@acardona
Copy link
Contributor Author

For testing I am using annotation "thermo-KC right" from Seymour, so only 4 KCs are loaded.

The 1099 volume has project_id=4.

What I did: login via psql to the catmaid_fibsem database and update the string in the class)_instance table:

select * from class where project_id=4 and class_name='landmark';
  id  | user_id | project_id |         creation_time         |         edition_time          | txid  | class_name | description 
------+---------+------------+-------------------------------+-------------------------------+-------+------------+-------------
 4745 |       2 |          4 | 2020-10-20 11:18:01.962822+00 | 2020-10-20 11:18:01.962822+00 | 44164 | landmark   | 
(1 row)
select * from class_instance where class_id=4745;
   id   | user_id | project_id |         creation_time         |         edition_time          |  txid   | class_id |                  name                  
--------+---------+------------+-------------------------------+-------------------------------+---------+----------+----------------------------------------
  10089 |       3 |          4 | 2021-01-30 09:54:38.979317+00 | 2021-01-30 09:54:38.979317+00 |  102133 |     4745 | NHO-d
  10090 |       3 |          4 | 2021-01-30 09:54:47.007144+00 | 2021-01-30 09:54:47.007144+00 |  102134 |     4745 | NHO-v
  10225 |       3 |          4 | 2021-01-30 10:30:11.996808+00 | 2021-01-30 10:30:11.996808+00 |  102250 |     4745 | SNR
  10297 |       3 |          4 | 2021-01-30 10:54:09.459283+00 | 2021-01-30 10:54:09.459283+00 |  102329 |     4745 | ISNR-a
  10355 |       3 |          4 | 2021-01-30 11:17:07.38662+00  | 2021-01-30 11:17:07.38662+00  |  102388 |     4745 | Nerve-Split
  25779 |       3 |          4 | 2021-06-07 07:47:34.922546+00 | 2021-06-07 07:47:34.922546+00 |  282807 |     4745 | MB Vertical Lobe Tip
  25781 |       3 |          4 | 2021-06-07 07:48:18.953372+00 | 2021-06-07 07:48:18.953372+00 |  282809 |     4745 | MB medial lobe tip
  51812 |       3 |          4 | 2022-01-27 13:33:35.627962+00 | 2022-01-27 13:33:35.627962+00 |  456200 |     4745 | ISNR-p
 329328 |       3 |          4 | 2024-09-17 10:36:13.463282+00 | 2024-09-17 10:36:13.463282+00 | 1927577 |     4745 | CPa/bl/f and MB Peduncle Closest Point
  62002 |       3 |          4 | 2022-05-24 09:40:09.502739+00 | 2025-01-27 11:56:42.604205+00 | 2412380 |     4745 | MB Vertical / Medial Lobe Intersection
(10 rows)

And finally:

update class_instance set name='MB Vertical / Medial Lobe Intersection' where class_id=4745 and id=62002;

And now 4 landmarks are used. There must have been an encoding error in the string, entirely invisible to the naked eye but javascript compared the two and returned false.

@acardona
Copy link
Contributor Author

An issue persists, though: while 4 landmakrs are now being used, the transformation is OK in the antero-posterior axis but is messed up medio-laterally: inverted or worse.

@acardona
Copy link
Contributor Author

Further testing shows that when projecting neurons in Seymour's annotation "thermo-KC right" using Seymour's landmark group "brain hemisphere right" to 1099's landmark group "brain hemisphere left" then the transformation is largely correct, barring differences in the individual landmark positions.

Whereas when projecting to 1099's "brain hemisphere right" then the neurons are medio-laterally flipped. I have run out of clues to follow to figure this out.

@acardona
Copy link
Contributor Author

acardona commented Jan 28, 2025

Here is a view of Seymour's annotation "thermo-KC right" projected to 1099 via "brain hemisphere right" (yellow) landmark group, and to "brain hemisphere left" (orange) landmark group. Notice how the right-to-right is medio-laterally flipped, whereas the right-to-left looks about correct:
(anterior view: right-to-right is on the left of the image, and right-to-left on the right side.)

Image

@acardona
Copy link
Contributor Author

To further add: I projected one KC from the left hemisphere of 1099 (annotation "KC left" with one neuron) to the right hemisphere, and the transformation is correct.

So the issue is likely related to projecting across projects: the landmark names and matching work fine locally across the hemispheres of 1099.

@acardona
Copy link
Contributor Author

More issues: when transforming right to right, I see at the end only 6 matches, where there should be 8.
There are 4 common landmarks between the two "brain hemisphere right" landmark groups, and the useReverseMatches is true. So there is one pair that is not being pushed into matches, despite sharedLandmarkIds having 4.

@acardona
Copy link
Contributor Author

acardona commented Jan 29, 2025

Found the cause: for the second pair, linkedFromLocationIdxs has a length of zero: it cannot find the landmark! It's the ID 21244857, name "MB medial lobe tip" for the Seymour volume. What's wrong with this landmark now?

Turns out, there is only 1 landmark under "MB medial lobe tip", assigned to Seymour's "brain hemisphere left" but not to right. This is what has caused so much trouble when matching with other volumes too.

@acardona
Copy link
Contributor Author

Finally, matches contains 8 entries. The right-to-left transform continues to be correct, but the right-to-right doesn't: there's something odd about the 3D coordinates of the landmarks themselves.

@acardona
Copy link
Contributor Author

To add that the GABA FIBSEM volume with pid=7 also seems to have problems: when projecting "thermo-KC right" from "brain hemisphere right" to "brain hemisphere right", one of the landmarks that matches by name somehow is not used.

Another issue is that the resulting transformation is rather wrong. May have to do with the landmarks themselves though, again.

@tomka
Copy link
Contributor

tomka commented Feb 17, 2025

Thanks for the server update. I just started looking into this again and to me it looks like your last example works now: if I add a transformation in the GABA FIBSEM volume from Seymour's "thermo-KC right" from its "brain hemisphere right" group to the FIBSEM's volume "brain hemisphere right" group, the resulting neurons look plausible to me in the 3D viewer. All four landmarks that match by name seem to be used in the MLS transform. Could you please try and see if this looks okay to you now? I saw the wrongly transformed last week as well and I am not sure any recent change by me fixed this. However the landmarks don't seem to have changed in either CATMAID instance either (as far as I can tell from the Landmarks widget). Did you change anything?

@tomka
Copy link
Contributor

tomka commented Feb 20, 2025

but the right-to-right doesn't: there's something odd about the 3D coordinates of the landmarks themselves.

I noticed that the right-to-right transform of "thermo-KC right" from Seymour to 1099 looks correct if "Use reverse point matches" is unchecked. The problem is likely that both source and target space overlap and that reverse matches will then cause "unlogical" mappings within the source space. This is likely only a problem in transformations between projects (since overlapping landmark groups don't happen usually within a project, it seems). The spaces don't overlap for the right-to-left transform, which is why the transformation is correct.

So far I assumed there is no harm to have "reverse matches" enabled by default, but given this error case, users should at least be warned: I could add a warning to the UI if "reverse matches" are enabled and the bounding boxes of all source landmarks and all target landmarks overlap.

tomka added a commit that referenced this issue Feb 25, 2025
…t BBs

If source and target bounding boxes overlap or touch, wrong or rather
surprising transformations can be the result if reverse matches are
enabled. This commit adds a warning for this situation to the displayed
list of current matches so that the user is aware about this.

See #2285
@tomka
Copy link
Contributor

tomka commented Feb 25, 2025

The Landmark Widget now displays a warning in the list of current matches, if reverse matches is enabled and the source and target bounding boxes overlap. Do you think this should be taken care of additionally in another way?

Is there anything else unexplained in this issue or can we mark this as done?

@acardona
Copy link
Contributor Author

Thanks Tom for all the fixes.

When I project "thermo-KC right", I choose as the source group "brain hemispheres right" and I get an error message that there isn't a matching group, yet there is. Are the two strings not written in the same encoding? Why does the comparison fail? Such an error was happening too among landmark names and was only resolved by updating the string directly in the database via psql.

@tomka
Copy link
Contributor

tomka commented Feb 26, 2025

Are you getting this error in 1099? If I select Seymour/L1 as source project and select "brain hemispheres right" as source group, "brain hemisphere right" is selected automatically as target group (if no target group was set manually before).

@acardona
Copy link
Contributor Author

It's in the 1128 (GABA) volume.

@tomka
Copy link
Contributor

tomka commented Feb 26, 2025

Indeed, the landmark groups "brain hemisphere left" and "brain hemisphere right" have a a TAB character as the first character ("\t"). We should probably trim all spaces around the group name during creation---I'll add that change.

tomka added a commit that referenced this issue Feb 26, 2025
Otherwise accidental tabs and whitespaces can prevent matching of
groups.

See #2285
@tomka
Copy link
Contributor

tomka commented Feb 26, 2025

For the landmarks themselves the user input was already trimmed. It should be quick to add a button that will trim all landmarks and groups (or you do this again in the DB).

@acardona
Copy link
Contributor Author

acardona commented Mar 4, 2025

Goodness, well spotted. Perhaps a button is best - would be dangerous to write many individual UPDATE commands in psql.

tomka added a commit that referenced this issue Mar 4, 2025
This button will instruct the back-end to ensure all landmark names and
landmark group names don't have any leading or trailing whitespace.

See #2285
@tomka
Copy link
Contributor

tomka commented Mar 4, 2025

I have added such a button to the Landmarks tab. It's called "Normalize names" and will ensure no leading or trailing whitespace is used in landmark names and landmark group names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants