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

Support for 64 bit integers as skids #114

Open
jefferis opened this issue Jun 4, 2019 · 7 comments
Open

Support for 64 bit integers as skids #114

jefferis opened this issue Jun 4, 2019 · 7 comments

Comments

@jefferis
Copy link
Collaborator

jefferis commented Jun 4, 2019

This is a bit of a pain to handle since base R can only handle signed integers up to 2147483647. Options are to use bit64 package, int64 package (prob not) or allow numerics (i.e. 64 bit doubles) and try to handle rounding issues. The main function affected is catmaid_skids. @mmc46

@jefferis
Copy link
Collaborator Author

jefferis commented Jun 4, 2019

@schlegelp can you confirm that there is now support for 64 bit skids in catmaid dev?

@tomka
Copy link

tomka commented Jun 4, 2019

@schlegelp can you confirm that there is now support for 64 bit skids in catmaid dev?

I can answer this: it is not part of dev yet, but I wrote the migration and will likely merge this next week after I cut a new CATMAID release.

This will also have the consequence that basically all IDs are 64 bit (e.g. annotations, tags, etc.).

@jefferis
Copy link
Collaborator Author

jefferis commented Jun 4, 2019

Thanks @tomka. I guess it was going to happen eventually but it’s going to be a minor headache.

@tomka
Copy link

tomka commented Jun 4, 2019

Just for the sake of completeness, this is the PR for CATMAID that adds 64 bit IDs for semantic data: catmaid/CATMAID#1887. Once this is merged, chances are it will be on a production server soon.

@jefferis
Copy link
Collaborator Author

So noting down just a couple thoughts here

  1. we'll probably need to audit every location that uses an id that could come in as 64 bit
  2. we essentially only need to worry about equality (and by extension fast searches) for ids (we don't need to worry about arithmetic and positional indexing)
  3. it might make sense to make some thin wrapper S3 class (e.g. catmaid_id) so that if we ever need to look at this again, we can do it in one place
  4. we could potentially get some benefit from this change by trying to provide better support for indexing e.g. neuronlists by ids (where we want to index as if they are character vectors not ints, which are interpreted as arrant indices not identifiers).

@jefferis
Copy link
Collaborator Author

See discussion https://stackoverflow.com/questions/35171760/in-r-is-it-better-to-use-integer64-numeric-or-character-for-large-integer-id-n for some of the issues (they vote for character vectors).

@jefferis
Copy link
Collaborator Author

jefferis commented Apr 6, 2020

Just a note that in neuprintr we accept 64 bit ints as ids and use them under the hood but have ended up returning ids as character vectors – this just ends up simpler in nearly all cases and the efficiency savings seem minor.

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

No branches or pull requests

2 participants