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

Update dependencies #235

Closed
wants to merge 17 commits into from
Closed

Update dependencies #235

wants to merge 17 commits into from

Conversation

adamltyson
Copy link
Member

@adamltyson adamltyson commented Jun 23, 2023

This PR starts the process of bringing the dependencies up to date, hopefully making brainrender easier to maintain. It also updates all the tooling used (testing, linting etc) to match the rest of brainglobe (again, hopefully to make maintenance easier).

However, a number of issues remain. One of the most obvious is that different actors are rendered in different places (see example below). I haven't managed to track down the issue, and I was hoping @FedeClaudi, that you may have an idea?

Example script to reproduce (no screenshot ATM because brainrender screenshots don't work on my system, and render() blocks my keyboard/mouse!:
import random
import numpy as np

from brainrender import Scene
from brainrender.actors import Points, PointsDensity


def get_n_random_points_in_region(region, N):
    """
    Gets N random points inside (or on the surface) of a mes
    """

    region_bounds = region.mesh.bounds()
    X = np.random.randint(region_bounds[0], region_bounds[1], size=10000)
    Y = np.random.randint(region_bounds[2], region_bounds[3], size=10000)
    Z = np.random.randint(region_bounds[4], region_bounds[5], size=10000)
    pts = [[x, y, z] for x, y, z in zip(X, Y, Z)]

    ipts = region.mesh.inside_points(pts).points()
    return np.vstack(random.choices(ipts, k=N))


scene = Scene(title="Labelled cells")

mos = scene.add_brain_region("MOs", alpha=0.15)
coordinates = get_n_random_points_in_region(mos, 2000)

# Add to scene
scene.add(Points(coordinates, name="CELLS", colors="salmon"))
scene.add(PointsDensity(coordinates))

scene.render()

@adamltyson adamltyson changed the title Version 2.1 Update dependencies Jun 26, 2023
@marcomusy
Copy link
Contributor

it looks like x and y coords are flipped wrt the obj files in ~/.brainglobe/allen_mouse_25um_v1.2/:

vedo ~/.brainglobe/allen_mouse_25um_v1.2/meshes/*.obj

Screenshot from 2023-06-27 17-35-02

vs brainrender:
Screenshot from 2023-06-27 17-36-50

@FedeClaudi
Copy link
Collaborator

Thanks @marcomusy

@adamltyson did something change in the way atlases are created, or in the atlas itself?
Previously we had to transform actors coordinates from meshes/data to make it match the coordinate system of the atlas, but that was more about L/R mirroring.
In the screenshot from Marco the whole atlas orientation seems different.

Also, why are the screenshots not working?
You can also try to render without interactive mode to see if that frees your mouse/keyboard to take a screenshot.

@adamltyson
Copy link
Member Author

@adamltyson did something change in the way atlases are created, or in the atlas itself?

As far as I know, nothing has changed. The Allen mouse atlas hasn't changed since 2020. I think everything is the correct way round in the released version on PyPI (it's just installation that's a problem).

Previously we had to transform actors coordinates from meshes/data to make it match the coordinate system of the atlas

I remember these conversations, so I hoped you might have an idea of what was going on. There seems to be two potential causes:

  • I changed something without realising (possible, but I tried to just update dependencies & tooling in this PR)
  • A dependency changed something (guessing not vedo or @marcomusy would have mentioned something)

I don't want to start messing with rescaling/reorienting things myself because:

  1. I assume this is a simple fix (if we can find it)
  2. I don't want to break things I don't understand.

I should also flag that there are other examples too, e.g. in the gene expression data, the gene expression lego blocks are rotated 90 deg wrt the brain.

@FedeClaudi
Copy link
Collaborator

But then I don't get why

vedo ~/.brainglobe/allen_mouse_25um_v1.2/meshes/*.obj

gives a brain with the left-right axis along z. Either something changed in the atlas or in vedo or one of its dependencies?
The transformations we applied before mirrored L/R but they shouldn't result in any rotations.

I should have some time this weekend to have a look. But if you use brainrender from the main branch do you still see these bugs?

@adamltyson
Copy link
Member Author

Either something changed in the atlas

I'm sure the atlas itself hasn't changed. I don't think anything relevant in the API has changed.

But if you use brainrender from the main branch do you still see these bugs?

TBH I didn't get it running on my machine without making the changes in this PR. I think the released version is ok, I haven't seen any bug reports.

@marcomusy
Copy link
Contributor

Hi, I cannot guarantee that something relevant to this issue has not change in vedo, in the

vedo ~/.brainglobe/allen_mouse_25um_v1.2/meshes/*.obj --alpha 0.1

you see the "original" coordinates in the 840 files without any transformation of any kind.. so the bounding box is

x=(-16.8, 1.32e+4) y=(134, 7.56e+3) z=(486, 1.09e+4)

I can find some time to look into brainrender too if you think i can help (after next week).

About the screenshot it may be a vtk thing (if so there is a work-around).

@adamltyson
Copy link
Member Author

#246 (comment)

@adamltyson
Copy link
Member Author

@alessandrofelder can this be closed in light of #250?

@alessandrofelder
Copy link
Member

Yes, I think so. (We shouldn't forget about the discussion here though!)

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

Successfully merging this pull request may close these issues.

4 participants