search

pecos / tps

Torch Plasma Simulator
BSD 3-Clause "New" or "Revised" License
8 stars 2 forks source link

Fix compiler warnings #232

Closed trevilo closed 1 year ago

trevilo commented 1 year ago

We have accumulated many compiler warnings. This PR fixes these, at least those generated in our automated CI environments. This PR also fixes some warnings occuring at ./bootstrap time.

In all the CI environments, we have added appropriate flags (e.g., -Werror for gcc) to force compiler warnings to be treated as errors. This should keep the code, at least changes merged to main, clear of warnings going forward.

uvilla commented 1 year ago

Thank you @trevilo for addressing this!

One quick comment, I would avoid using the method mfem::Vector::GetData.

This method is present in mfem for legacy reasons (before GPU support was added). However, it is very brittle when used in hybrid cpu/GPU codes as it does not ensure data consistency between the host and the device.

instead I would recommend to ask for the raw pointer (possibly outside any for loop to avoid overheads) using mfem::Vector::HostRead (if you just need a const double * for read access only), mfem::Vector::HostReadWrite (if you want to read values but the interface require a non const pointer) and finallmfem::Vector::HostWrite (if you are only overwriting values without need to first read them).

trevilo commented 1 year ago

Thanks @uvilla. Yeah, I'm aware of the HostReadWrite etc. The only places that were changed to GetData were in code that was previously using an implicit conversion from mfem::Vector to double *. I know that this code will never run on the device and that the variables in question will never have their data on the device.

But, regardless of that, you are right. It is far better to not perpetuate this usage. I will fix the ones I added here. Purging the rest of the code of GetData is a larger effort that I won't put in this PR.