search

vgteam / xg

xg0: a simpler xg index
Other
5 stars 4 forks source link

Revert to default implementation of for_each_step_position_on_handle #28

Closed glennhickey closed 4 years ago

glennhickey commented 4 years ago

xg re-implements the non-pure-virtual handlegraph api method for_each_step_position_on_handle(). But xg's version doesn't seem to work all the time.

On this graph: https://github.com/vgteam/vg/blob/master/src/unittest/chunker.cpp#L19-L80 Querying the positions on node 4 on path y with this function gives 11-forward and 36-reverse using xg's implementation. With the default implementation, which uses xg's own get_step_at_position() function, the result is 11-forward and 31-reverse (which is what I get when computing by hand). When built on Mac/clang, xg seems to get it right though (this whole thing came up when debugging why Travis test results are different for https://github.com/vgteam/vg/pull/2506 on the Mac).

Anyway, given that XG::for_each_step_position_on_handle() doesn't seem more performant than the default implementation, I'm just getting rid of it, rather than worry about why it doesnt' work.

ekg commented 4 years ago

Thanks Glenn!

On Sat, Dec 7, 2019, 13:41 Glenn Hickey notifications@github.com wrote:

Merged #28 https://github.com/vgteam/xg/pull/28 into master.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vgteam/xg/pull/28?email_source=notifications&email_token=AABDQENP2YBRC7RDT4MY7LLQXOKX3A5CNFSM4JW6DUF2YY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOVKWXWKA#event-2863495976, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABDQEIMRSCLKMFX76VLQLLQXOKX3ANCNFSM4JW6DUFQ .