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

Time Remaining overflow error #77

Open
csparker247 opened this issue Nov 12, 2020 · 2 comments
Open

Time Remaining overflow error #77

csparker247 opened this issue Nov 12, 2020 · 2 comments
Labels
bug Something isn't working

Comments

@csparker247
Copy link
Contributor

I'm seeing an overflow issue with the Time Remaining display for progress bars with long run times and large progress values. The Time Remaining starts at a reasonable run time (lets say 7 hours), decreases normally for a bit, then jumps down to zero and begins counting up. At some point, it resets back to zero and begins counting up again. This final cycle repeats for the duration of the process.

It takes awhile for my process to start up, so it's been hard to capture an example of the beginning state, but here's what it looks like when it cyclically resets back to zero:

indicator-reset

This seems like an integer overflow scenario in progress_bar.hpp:257:

        auto eta = std::chrono::nanoseconds(
            progress_ > 0 ? static_cast<long long>(elapsed_.count() *
                                                   max_progress / progress_)
                          : 0);

It doesn't take long (in nanoseconds) for elapsed_.count() * 1325697865 to overflow long long and my specific scenario can be quickly fixed by enforcing a specific order of operations: elapsed_.count() * (max_progress / progress_). I'm happy to submit this as a patch, but this only shifts the issue to an even larger value of max_progress. Should there also be some mention of this issue in the documentation?

@csparker247
Copy link
Contributor Author

As an alternative here, I wonder if elapsed_ should actually be something like std::chrono::duration<long long>
or std::chrono::duration<long long, std::milli> since we only report durations to the second.

@p-ranav
Copy link
Owner

p-ranav commented Nov 12, 2020

Ideally, the tick resolution can be added to the configuration, e.g.,

ProgressBar bar {
  // ...,
  option::TickResolution{TickResolution::milliseconds}
};

That way, the user can configure this setting and it'll work for long-running tasks where the remaining time is in order of minutes.

@p-ranav p-ranav added the bug Something isn't working label Nov 12, 2020
csparker247 pushed a commit to educelab/volume-cartographer that referenced this issue Jan 7, 2021
(Texturing) Add ThicknessTexture algorithm

### Thickness texturing

New algorithm that textures a layer based on its local thickness. Requires a `volume-mask` generated by the TFF algorithm. Algorithm is supported in both `vc_render` and `vc_render_from_ppm`. To get the raw thickness counts, save as a TIFF and use flags: `--normalize-output false --tiff-floating-point`.

Example workflow:
```shell
# Run TFF segmentation. Currently outputs pointset.vcps in working dir
vc_segment -v Test.volpkg -s 20210105155425 -m tff --stride 100 --save-mask volmask.vcps

# Convert to PLY point cloud
vc_convert_pointset -i pointset.vcps -o pointset.ply
# Use MeshLab to clean and mesh the point cloud -> mesh.obj

# Run thickness texture
vc_render -v Test.volpkg --input-mesh mesh.obj -o thickness.tif --normalize-output false --tiff-floating-point -m 3 --volume-mask volmask.vcps
```

### Other changes
#### Apps and app_support
- Update `indicators` subproject to fix [overflow bugs](p-ranav/indicators#77)
- (vc_segment) Remove final references to STPS algorithm
- (vc_render_from_ppm) Use some shared render options from `vc_render`
- (vc_render[_from_ppm]) Correctly save jpg images in 8bpc
#### Testing
- Update Google Test version to silence CMake deprecation warning
#### Core
- **New:** `VolumetricMask` class for tracking per-voxel masking information. Infinitely faster than `VolumeMask`, but much more memory-greedy. Keeping both implementations around for now.
- (PointSetIO) Let `ReadPointSet` load `OrderedPointSet` files as `PointSet`
- (PerPixelMap) Add cell map functionality. A cell map records the per-pixel face assignment calculated by PPMGenerator.
- (HashFunctions) Add project-wide 3D vector hash function. Really only useful for integer-types, but templated.
- (ImageConversion) MinMax scaling on QuantizeImage can now be disabled
#### Segmentation
- Refactor useful flood fill functionality into standalone functions
- **New:** `ComputeVolumetricMask` for generating a VolumetricMask from an existing segmentation
#### Utilities
- (CannySegment) Add option to use outer contour of canny edges and to save all edges rather than just projected ones.
- **New:** `vc_convert_mask` bidirectionally converts between point masks (`.vcps`) and image sequences
- (ConvertPointSet) Enable bidirectional conversion between `.vcps` and `.ply/.obj` files
- **New:** `vc_image_stats` Calculate descriptive statistics over labeled regions of images
- (MergePointSets) Add ability to specify individual input files and specific output file path. Add `overwrite-overlap` option.
- **New:** `vc_seg_to_pointmask` Generate a VolumetricMask file from an existing segmentation using `ComputeVolumetricMask`
- **New:** `vc_visualize_ppm` Generate position map and normal map images from a PPM

See merge request seales-research/volume-cartographer!309
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants