Change G3VectorInt to wrap vector<int64_t> #42
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This allows G3VectorInt to properly store any valid G3Int, which was not previously the case. The G3Vector serialization for int64_t has been specialized and split (load/save) and is implemented in G3Vector.cxx This passes all core tests, which includes loading v1 vectors.
Updates portability.py to include v2, at least for LE. New test "vecint.py" to confirm full range support and compression work.
nwhitehorn
approved these changes
Nov 16, 2020
There was a problem hiding this comment.
This looks great to me! Thanks a bunch for doing it. I don't have any comments or complaints about the patch. Feel free to merge once Sasha gives her OK as well.
From the SPT side, the only obvious negative impact to me is that this will increase RAM usage for pixel indices when using our old, deprecated map binner. But not by a huge amount and the map binner in question is one we are actively trying to remove, so that doesn't seem like a huge problem.
arahlin
approved these changes
Nov 16, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This allows G3VectorInt to properly store any valid G3Int, which was
not previously the case. The G3Vector serialization for int64_t has
been specialized and split (load/save) and is implemented in
G3Vector.cxx.
The unnecessary complication I have added is some basic
compression-on-serialization, where a vector that can be safely
represented as int8/16/32/64 will be stored as that.
Includes a new test and a new "portability" test.