Improve timers and read_grid_cat restart

cholla-hydro / cholla

A GPU-based hydro code

https://github.com/cholla-hydro/cholla/wiki

MIT License

60 stars 32 forks source link

Improve timers and read_grid_cat restart #291

Closed alwinm closed 1 year ago

alwinm commented 1 year ago

Make minor fixes so that Cholla compiles without -DOUTPUT and -DHDF5
Add Hydro Integrator-specific timer. Note that double-counting time is not an issue since the Total is not computed from any sum.
Add CudaSynchronize to timers (necessary for Hydro Integrator)
Make slight improvement to Read_Cat_Grid to avoid overwriting input and to start at the correct nfile.
Make slight improvement to Read_Cat_Grid to correctly set some variables which are not strictly necessary but may cause confusion.
Change particle ngrid calculations to consistently use (number - 1) / TPB + 1. This has the benefit of never launching 0 blocks, which avoids cuda bug.

bcaddy commented 1 year ago

Do the additional cudaDeviceSync calls impact performance?

alwinm commented 1 year ago

Do the additional cudaDeviceSync calls impact performance?

It's a good question. I haven't looked carefully but it's likely sub-ms or within noise. The more impactful part of the CPU_TIME flag is the MPI collection and synchronization. To squeeze out all the performance juice I would recommend leaving the flag disabled.

alwinm commented 1 year ago

Overhead of timers, measured by placing a Start and End adjacent with nothing in between.

0.0052   ms

bcaddy commented 1 year ago

It looks like the MHD restart test is failing with this message

C++ exception with description "filesystem error: cannot rename: No such file or directory [/ihome/sam/jenkins/eschneider/eschneider/cholla-automated-testing/PR-291/hydro/run_output.log] [/ihome/sam/jenkins/eschneider/eschneider/cholla-automated-testing/PR-291/hydro/bin/tHYDROtMHDReadGridHdf5_RestartSlowWaveExpectCorrectOutput/run_output.log]" thrown in the test body.

This indicates that it can't find the run_output.log file. I'm not sure if this is just a one off error or if the filename was changed or something similar.

bcaddy commented 1 year ago

All the tests are passing now. Looks like it was a one off issue