Floating point events + image reconstruction additional arg #8
Conversation
danielgehrig18
commented
Jun 3, 2021
danielgehrig18
commented
- implemented events with floating point coords
- changed int64 to uint32
- implemented numbering of reconstructions according to index
- implemented numbering of reconstructions according to index
There was a problem hiding this comment.
I don't think that we can consider uint32 to be safe. There is no guarantee that the timestamps from a rosbag are small enough to avoid overflow.
An alternative would be to have t as uint32 and have a uint64 offset. But then we need additional code to reconstruct the true timestamp.
Have you checked the file size difference if compression is used? Does it matter a lot? If yes, we could also consider more advanced compression like zstd.
Finally, the current code will break other code which explicitly constructs Event dataclasses with timestamp dtype of int64. E.g.
e2calib/python/conversion/ros.py
Line 27 in 657608d
or
e2calib/python/conversion/prophesee.py
Line 23 in 657608d
python/data/format.py
Outdated
| @@ -17,7 +17,32 @@ def __post_init__(self): | |||
| assert self.x.dtype == np.uint16 | |||
| assert self.y.dtype == np.uint16 | |||
| assert self.p.dtype == np.uint8 | |||
| assert self.t.dtype == np.int64 | |||
| assert self.t.dtype == np.uint32 | |||
There was a problem hiding this comment.
This is dangerous since we don't check that an overflow is not happening.
There was a problem hiding this comment.
Same for other loc that are affected by this
There was a problem hiding this comment.
Ok I am conviced, I have changed it back to int64. The memory footprint then grows from 12MB -> 13MB, which should be fine.
python/offline_reconstruction.py
Outdated
| @@ -38,6 +38,7 @@ def download_checkpoint(path_to_model): | |||
| parser.add_argument('--timestamps_file', '-tsf', help='Path to txt file containing image reconstruction timestamps') | |||
| parser.add_argument('--upsample_rate', '-u', type=int, default=1, help='Multiplies the number of reconstructions, which effectively lowers the time window of events for E2VID. These intermediate reconstructions will not be saved to disk.') | |||
| parser.add_argument('--verbose', '-v', action='store_true', default=False, help='Verbose output') | |||
| parser.add_argument('--index_by_order', '-i', action='store_true', default=False, help='Index reconstrutions with 0,1,2,3...') | |||
There was a problem hiding this comment.
default is False anyway for store_true
There was a problem hiding this comment.
fixed this with the latest commit
