Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add compile flag for supporting 64 bit IDs #1528

Closed
jeromekelleher opened this issue Jun 25, 2021 · 1 comment · Fixed by #1531
Closed

Add compile flag for supporting 64 bit IDs #1528

jeromekelleher opened this issue Jun 25, 2021 · 1 comment · Fixed by #1531
Assignees
Milestone

Comments

@jeromekelleher
Copy link
Member

The discussion in #343 makes it clear that it is likely that we will need to support very large tables at some point in the future, and so it would be wise to plan ahead for this. My suggestion for this is the following:

  • Add some defines that allow us to build the tskit library with 64 bit IDs (default is definitely 32 bit). Do not change any of the storage types in the file format, this is purely just about the size of the tsk_id_t and tsk_size_t types. Do not document this, and state clearly in the source code that this is not a supported configuration.
  • Add a step to the CircleCI config that builds the C library like this and runs the tests.

This way we'll be able to keep the library in 64bit readiness and should be able to make the jump fairly easily if/when we need to. Running the tests in 64bit mode in CI should prevent us from going down any 32 bit only rabbit holes.

Any objections?

@jeromekelleher jeromekelleher self-assigned this Jun 25, 2021
@jeromekelleher jeromekelleher added this to the C API 1.0.0 milestone Jun 25, 2021
jeromekelleher added a commit to jeromekelleher/tskit that referenced this issue Jun 28, 2021
jeromekelleher added a commit to jeromekelleher/tskit that referenced this issue Jun 28, 2021
@benjeffery
Copy link
Member

Sounds good - I assume the define might have to disable some of the file format tests.

jeromekelleher added a commit to jeromekelleher/tskit that referenced this issue Jun 28, 2021
jeromekelleher added a commit to jeromekelleher/tskit that referenced this issue Jun 28, 2021
jeromekelleher added a commit to jeromekelleher/tskit that referenced this issue Jun 28, 2021
@mergify mergify bot closed this as completed in #1531 Jun 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants