It is not clear if node_at_vector_offset/node_vector_offset are supposed to be 0-based, 1-based, or a mix.

[adamnovak]

The vg Sampler samples from the sequence using a 1-based offset:

https://github.com/vgteam/vg/blob/e558c3cc27984415ba3de7c3d941827c39f17376/src/sampler.cpp#L59-L62

However, this isn't documented in the VectorizableHandleGraph interface, so it is likely that implementers will produce and expect 0-based offsets.

It looks like the VectorizableOverlay from libbdsg produces 0-based offsets, but expects 1-based offsets, which is just wild:

https://github.com/vgteam/libbdsg/blob/e073c2aee77e73ec41310bf3191af322fe8faa7e/src/vectorizable_overlays.cpp#L88-L96

We should choose all 0-based or all 1-based (probably all 0-based), document it, and change all the clients and implementations.

[adamnovak]

Similarly, our vg Packer code expects edge_index to never return 0 for an edge that exists but this isn't guaranteed by the interface.

[glennhickey]

Some background here:

https://github.com/vgteam/libbdsg/pull/20

There was an inconsistency in xg that propagated into the vectorizable overlay. It was during the big terrible xg0 merge, so fixing it properly got pushed back until that was finished. No more excuses now though!

On Mon, Jan 6, 2020 at 8:09 PM Adam Novak notifications@github.com wrote:

Similarly, our vg Packer code expects edge_index to never return 0, for an edge that exists but this isn't guaranteed by the interface.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vgteam/libhandlegraph/issues/41?email_source=notifications&email_token=AAG373RKRH6RLPZ57KNSJTDQ4PI6LA5CNFSM4KDOFJJKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIHKXYI#issuecomment-571386849, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG373UYTCHWRZF45S3ASUDQ4PI6LANCNFSM4KDOFJJA .