New bitflag error

[hyanwong]

When using tsinfer I'm now getting the following error:

../../../miniforge3/envs/tsdate/lib/python3.12/site-packages/tsinfer/inference.py:1965: in get_samples_tree_sequence
    new_flags = np.bitwise_and(tables.nodes.flags, ~tskit.NODE_IS_SAMPLE)
E   OverflowError: Python integer -2 out of bounds for uint32

I think we might need to coerce tskit.NODE_IS_SAMPLE to the correct dtype, i.e.

new_flags = np.bitwise_and(tables.nodes.flags, ~tables.nodes.flags.dtype.type(tskit.NODE_IS_SAMPLE))

[hyanwong]

Getting this on CI now too, when trying to build tsdate:

https://app.circleci.com/pipelines/github/tskit-dev/tsdate/1222/workflows/87caff0a-1dbd-4fc3-9f73-095a32db96a6/jobs/1217

[benjeffery]

Hmm, looks like we need an additional test, as this wasn't hit on tsinfer's CI.

[hyanwong]

Weird, as this path should always be hit, I think. I wonder if it's some strange numpy/python version combo?

Either way, I think we should push out a bugfix release, as it's blocking other development. I am slightly yearning to get #931 and #928 into that release too, as both are approved and would help with tsdate analysis.

[hyanwong]

I'll see if I can get the test suite to fail on my local setup, then push a fix.

[jeromekelleher]

I think the solution here is probably to push an update to tskit where we export NODE_IS_SAMPLE as a numpy uint32, not a standard Python integer.

[hyanwong]

I think the solution here is probably to push an update to tskit where we export NODE_IS_SAMPLE as a numpy uint32, not a standard Python integer.

Yes, I think that would be the best long-term fix (and I guess for the other flags too?)

When I install tsinfer locally, it is still using numpy 1.26, because of sgkit pinning, and I wonder if this bug is only on numpy 2, hence it not being caught in CI currently.

[hyanwong]

I think the solution here is probably to push an update to tskit where we export NODE_IS_SAMPLE as a numpy uint32, not a standard Python integer.

Since the fix to get tsinfer to pass is easy, and has no perf implications (just called once at the top level of get_samples_tree_sequence, I wonder if we should have a belt and braces approach, and fix it on this one line of tsinfer (that uses bitwise negation, ~), so that tsinfer+numpy2 works even with the current version of tskit? There's no harm I can see in doing .dtype.type(tskit.NODE_IS_SAMPLE) even if tskit.NODE_IS_SAMPLE is already of the correct dtype, so it would always be backwards compatible.

[jeromekelleher]

Sure

[jeromekelleher]

But yes, it looks like all flags values should be exported as numpy uint32 values now because of changes in numpy 2.