Optimized implementation of the likelihood #22
Conversation
…me now passed as an argument
PR approved?
PR approved?
all errors should be fixed now.
…tive paths, created modified_io, added 1e-10 to return value of likelihood function
… on _tree_utils_new.py
There was a problem hiding this comment.
Hi Laurenz,
Very nice improvements! To provide some constructive criticism, I think it'd be good to unify the code – there are several backends, and although they are very nice, I think in the long-term we won't be able to maintain them.
Probably the best way forward would be to just select one of them (e.g., the one that has good speed and the cleanest code) and remove the others (and the warmup scripts used to test them).
Perhaps the cleanest option (in terms of Git-jitsu) to do this is to open a new branch from this one, do the changes, and then merge that branch into this one, which will in turn later be merged into main. Some other general tips on maintaining Git history can be found there.
|
Hi Pawel, I attempted to implement Xiang's idea. However, I'm not sure if I did it exactly the way you wanted. Regardless, the runtime for calculating the likelihood of 623 trees is now at approximately 0.8 seconds. There seems to be a consistent overhead of 1.7 seconds when running the "R_py_loglikelihood_comparison" files. This overhead stems from the Unix time command, which measures not only the direct runtime of the program but also other operations. Additionally, the time measurement for CSV-Numpy conversions, tree-parsing etc. contribute to this overhead. Therefore, my first implementation of the likelihood had a runtime of slightly more than 10 seconds (not 12). The _backend.py and _tree_utils.py files contain the implementation of the likelihood from the last commit, while the _backend_geno.py and _tree_utils_geno.py files contain the updated version. I also started working on the report and I played around with the examples you recommended on Bayesian statistics. However, I haven't quite finished Michael Betancourt’s "Inferring gravity from data". See you on Tuesday and sorry for the late reply! |
|
Hi Laurenz! Great work! To respond to the individual items:
0.8 seconds is truly amazing!
Very nice 🙂
Wonderful! Looking forward to hearing what you think 🙂
Don't worry! It's a long exercises and I didn't expect you'd finish it in just a week!
Please, please, remember to have nice rest and not work on weekends. The thesis is a marathon not a sprint, as my older PhD friends tell me 😉 Great work and see you tomorrow! |
|
Hi Laurenz, I think it'd be good to merge this branch soon (otherwise the merge conflicts with #23 may get even harder to resolve). I thought I could be helpful here, so I:
I wonder if you could take a look at these changes. If they look fine, please merge 🙂 |
I attempted to implement Xiang's indexing idea.
The runtime for calculating the likelihood of 623 trees is now at approximately 0.8 seconds (the first implementation of the likelihood had a runtime of slightly more than 10 seconds).
The
_backend.pyand_tree_utils.pyfiles contain the implementation of the likelihood from the last commit, while the_backend_geno.pyand_tree_utils_geno.pyfiles contain the updated version.