Use ZFP for compressing discrete trajectories #2485
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
eggrobin
reviewed
Mar 8, 2020
physics/discrete_trajectory_body.hpp
Outdated
| downsampling_.has_value() ? downsampling_->tolerance() : Length(); | ||
| ZfpCompressor length_compressor(length_tolerance / Metre); | ||
| // Speeds are approximated based on the length tolerance and the average step | ||
| // in the timeline. |
There was a problem hiding this comment.
Let us not average things over the timeline; this is meaningless (the timeline may behave completely differently at different times) and it might result in weird edge cases that are hard to find and test, e.g., an interplanetary trajectory (large Δt, thus requiring a small speed tolerance to keep the error of the Hermite interpolation in check) getting mangled by a long time spent in low orbit, or even just by a lot of dense intervals (low Δt, 10 s for dense intervals, driving the mean Δt down).
Use the max Δt.
eggrobin
reviewed
Mar 12, 2020
serialization/physics.proto
Outdated
| optional int32 zfp_codec_version = 7; | ||
| optional int32 zfp_library_version = 8; | ||
| optional bytes zfp_timeline = 5; | ||
| optional int32 zfp_timeline_size = 6; |
There was a problem hiding this comment.
These should be wrapped in a message, since they are really optional and none or all should be present.
eggrobin
approved these changes
Mar 12, 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 seems to buy us a factor 4 in size, while complying with the downsampling tolerance of 10 m.
Helps with #2400.