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

Tree saving available? #84

Closed
KYQiu21 opened this issue Oct 2, 2023 · 14 comments · Fixed by #88
Closed

Tree saving available? #84

KYQiu21 opened this issue Oct 2, 2023 · 14 comments · Fixed by #88

Comments

@KYQiu21
Copy link

KYQiu21 commented Oct 2, 2023

Dear developers,

Thanks very much for this package. I am wondering if the current version supports saving/writing a tree.

Kaiyu

@mkborregaard
Copy link
Member

It is not currently available to write a tree to a Newick file (only read), though you can always serialize your tree with JLSO (for future use in Julia). A Newick writer would be nice though.

@mkborregaard
Copy link
Member

I guess a workaround is to use the R interface to write the tree.

@KYQiu21
Copy link
Author

KYQiu21 commented Oct 22, 2023

Thanks!

@richardreeve
Copy link
Member

@KYQiu21 In fact, I'm beginning a full rewrite of the parser for reading newick / nexus files, and as part of that it's my intention to write a tree "writer" as well. I can currently parse the trees, but not actually input them, so I've got a bit of a way to go, but I hope to have time this month to complete this work.

@KYQiu21
Copy link
Author

KYQiu21 commented Nov 10, 2023

Thanks for your effort and look forward to a more powerful version, although it is already excellent 😉

@richardreeve
Copy link
Member

Just to update here, I have been working on the parser, but I spotted some things that were slowing it down in the main code, and with the latest (0.5) release I can now read the largest supertree of plants that I'm aware of (31k tips) in 293ms rather than 23s (about 80x faster). Unfortunately this fix took some time (and may be lightly breaking for some people because I change the underlying tree structure to make type inference work, hence the 0.5 release) so I haven't made the progress I was hoping for on the main parser. I'm starting on that now.

@mkborregaard
Copy link
Member

Just stating the obvious here: reading a 31k tips supertree in 293 ms (and building the corresponding representation in memory) in 293 ms is a pretty fantastic achievement. The plotting of that tree with this package would also be so much faster than R than even TTFP does not matter.

@KYQiu21
Copy link
Author

KYQiu21 commented Dec 8, 2023

Indeed!

@richardreeve
Copy link
Member

Thanks. It's a nice thought, but I just tried benchmarking ape's read.tree(), and it's 86ms in RStudio (or perhaps unsurprisingly a faster 75ms using RCall and BenchmarkTools in Julia), so I've got a way to go still. Nonetheless, it's a lot closer!

To be fair they have been developing ape for over 20 years, and it's mostly written in C...

@richardreeve
Copy link
Member

I just wish there was a decent parser generator for Julia. I've previously used yacc and bison in C, and it's just agonising handwriting them using the tools we currently have in Julia.

@mkborregaard
Copy link
Member

Yeah this is something the biojulia people, e.g. @jakobnissen are also battling with (and improving).

@mkborregaard
Copy link
Member

I think ape is faster because it doesn't build any internal representation - it's just two vectors of node links?

@richardreeve
Copy link
Member

Yes. that does make things simpler, but I've now written a parser using ParserCombinator.jl, and just parsing the 31k tip tree takes a few seconds without generating any structure sadly.

@richardreeve
Copy link
Member

richardreeve commented Dec 11, 2023

Yeah this is something the biojulia people, e.g. @jakobnissen are also battling with (and improving).

Unfortunately they aren't though as far as I understand it, at least not in this context - BioJulia/Automa.jl#59 makes it clear they don't believe recursion is in scope, at least for the time being.

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 a pull request may close this issue.

3 participants