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

Experimenting with refactoring tb #345

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

theosanderson
Copy link
Owner

No description provided.

@vercel
Copy link

vercel bot commented Aug 18, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cov2tree ✅ Ready (Inspect) Visit Preview Aug 22, 2022 at 2:21PM (UTC)

@theosanderson
Copy link
Owner Author

Hey @amkram,

I've broken a ton of stuff here to make a very basic proof of concept.

Basically the idea is:

  • don't do custom filtering for the treenome browser, use the trees from the main site which are already filtered. But atm I have only implemented the "overview", not the detailed tree when you zoom in (which is obviously crucial).
  • instead of drawing over things, create an array of line segments which misses out sections that need to be missed out because of revertant mutations, etc.

I'd be interested to know if this is an approach you started with and discarded, and generally any concerns you have about it

@theosanderson
Copy link
Owner Author

(at present mutations back to reference show up as their own mutations, but we can filter those out)

@amkram
Copy link
Collaborator

amkram commented Aug 18, 2022

Hey, thanks for getting started on this refactor! Sorry that some of my code is a bit messy.

A couple of thoughts:

don't do custom filtering for the treenome browser, use the trees from the main site which are already filtered. But atm I have only implemented the "overview", not the detailed tree when you zoom in (which is obviously crucial).

I might not fully understand how filtering works in Taxonium / the refactor, but right now treenome considers data.data.nodes as a filtered subtree, then does further filtering based on vertical height to avoid rendering lines that would be invisible anyway when zoomed out. I found that without that further filtering there is some lag in the zoomed out version of data.data.nodes. I might need some clarification on this new approach - but it seems to work well in the zoomed out version.

instead of drawing over things, create an array of line segments which misses out sections that need to be missed out because of revertant mutations, etc.

This would be great, and make things like implementing highlighting a lot easier. I think one reason I avoided doing this was to avoid increasing the polygon count, which had caused me some issues in the past. But navigating the browser seems pretty responsive in the PR demo, so I think we should be good!

@theosanderson
Copy link
Owner Author

Thanks! I don't think you're missing anything, thanks for explaining everything. The first point may well be an issue when I get the non-zoomed out version in - let's see. Thanks for the context!

@theosanderson
Copy link
Owner Author

(and it's not messy! definitely not in comparison to everything else in the codebase :) )

@theosanderson
Copy link
Owner Author

Another recent initially hacky change to try to use modelMatrix to handle X positioning, rather than doing this on the fly and updating the layer. I think this makes a substantial diff to performance (but is atm cancelled out by my removal of extra filtering, maybe time to add that back).

@theosanderson theosanderson changed the title Experimenting with refactoring treenome Experimenting with refactoring tb Sep 26, 2022
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.

2 participants