voxcraft / voxcraft-sim

a GPU-accelerated voxel-based physics engine
Creative Commons Zero v1.0 Universal
43 stars 10 forks source link

increases cudaLimitPrintfFifoSize 50 times from default #52

Closed mertan-a closed 2 years ago

mertan-a commented 2 years ago

the print calls from device are stored in a circular buffer before being flushed. when the amount of content exceeds the limit of the buffer, it starts overwriting the oldest content. to work around that, this commit does the following:

I believe this can solve the issue in most cases, but still is probably not the best way to do it. let me know if you prefer a different way (take the new size as a command line parameter, have a fixed size for the buffer, etc)

davidmatthews1uvm commented 2 years ago

Lgtm. Maybe we could reuse the VXA files for the print buffer just like how the gpu heap is specified. E.g. https://github.com/voxcraft/voxcraft-sim/blob/dfb61f7c8fcc1a4f9277d2fcd98f721d2a6adbf1/src/VX3/VX3_SimulationManager.cu#L369

davidmatthews1uvm commented 2 years ago

Thank you @mertan-a !!!!!!

Before merging, could you please add a tag to the simulationManager as @davidmatthews1uvm suggested so it's adjustable in the VXA?

Should it be 1 by default?

Thanks for contributing this.

Maybe the field should be in units of megabytes so that if cuda changes the default buffer size in the future, voxcraft does not have unexpected behavior.

A good default might be somewhere in the range of: 50-500 MB

@skriegman what are your thoughts on the default size / units?

mertan-a commented 2 years ago

Somehow I missed that tag, I'll make the change.

About default size and unit: I was initially thinking MB would be better. However, upon experimenting, I realized that you don't get the exact size you asked for (see https://forums.developer.nvidia.com/t/size-of-printf-buffer/160957 ). Now using MB could save us from changes in default buffer size, but is confusing because you don't get what you asked for. Using a multiplier applied to the default size is less confusing (if your history file is corrupted, just increase the multiplier), but a change in default size could create problems. I can do both, waiting for your comments @skriegman @davidmatthews1uvm

skriegman commented 2 years ago

I think the multiplier will be fine then.