Open jimallman opened 8 years ago
jar398
commented
8 years ago I believe there are only two studies fitting this description: pg_1766 and pg_2870. They can be fixed in the curator app.
for f in `find study -name "*.json"`; do if grep -q '"^ot:branchLengthMode": "ot:time",' $f; then if ! grep -q Myr $f; then echo $f; fi; fi; done
study/ot_68/ot_768/ot_768.json
study/pg_66/pg_1766/pg_1766.json
study/pg_70/pg_2870/pg_2870.jsonI checked these two studies. The trees in question appear not to have branch lengths at all, or else all branches have the same length. At least that's how it looks in the tree viewer. So I'm inclined to say that having the mode be ot:time is wrong in both cases. Not sure how best to fix, but setting the unit to Myr is almost certainly wrong. Anyone have suggestions?
snacktavish
commented
3 years ago It looks like, may be due to this original bug, we have many studies which have "^ot:branchLengthTimeUnit": "Myr", even though the branch lengths are not in terms of time, e.g. "^ot:branchLengthMode": "ot:changesCount",
snacktavish
commented
3 years ago The most recent example I can find is: https://github.com/OpenTreeOfLife/phylesystem-1/blob/master/study/ot_29/ot_729/ot_729.json Which suggests this may have been fixed in 2016... but maybe was actually in issue in the treebase import? I'm having trouble digging up the treebase nexml directly... I think it should be here: http://purl.org/phylo/treebase/phylows/study/TB2:S18336?format=nexml but it isn't
jimallman
commented
3 years ago It looks like, may be due to this original bug, we have many studies which have "^ot:branchLengthTimeUnit": "Myr", even though the branch lengths are not in terms of time, e.g. "^ot:branchLengthMode": "ot:changesCount",
This doesn't quite fit the history of fixes, since the original bug was just about having empty values for the expected time unit. It's possible that this is happening if someone chooses a time-based mode, then chooses changesCount or whatever. I'll check to see if the UI is smart enough to clear the time-units field in that case.
Or, we reason that the time-units value is moot (ignored) if the branch-length mode is not time-based..?
snacktavish
commented
3 years ago Yep - it looks like if you click on time, and the then go back, it gets set!
https://github.com/OpenTreeOfLife/phylesystem-0/commit/92de900a84cf354b55b7a220334ac402140cc6f9
Due to a minor UI bug (now fixed), this property was not being set in the web-app UI, and in many cases it will be empty. We might want to sweep the phylesystem repo for any trees whose
ot:branchLengthModeis set toot:time, and (if this unit is still empty) assert "Myr" instead.N.B. As of today (June 2, 2016) "Myr" has been the only valid choice for this property, but there will soon be other units available. So this change should only be done for older trees, if at all.