cholla-hydro / cholla

A GPU-based hydro code
https://github.com/cholla-hydro/cholla/wiki
MIT License
60 stars 32 forks source link

Updates to enable compilation of chemistry_gpu. #388

Closed brantr closed 5 days ago

brantr commented 3 months ago

Various minor issues prevented chemistry_gpu from compiling, and minor changes are needed to update this part of the code to account for the temperature floor implementation. This update has been confirmed to compile (on Frontier) and can now be used for validating the cosmological simulations run with dev that include chemistry and cooling.

evaneschneider commented 1 week ago

I've rerun the tests on this one, and the only thing that is failing is clang tidy, because the Chemistry_Header struct doesn't meet our style guidelines, which specify pascal case for structs (so it should be ChemistryHeader). See the summary at the very bottom of this discussion https://github.com/cholla-hydro/cholla/discussions/206.

I apologize that this is not in a more prominent place - adding style guidelines to the PR instructions on the wiki will go on my to-do list.

brantr commented 6 days ago

@evaneschneider I think this branch fails to build because of changes to the reconstruction when-DSCALAR is set, not because of changes I made. The dev branch currently does not compile for me when -DSCALAR is set (e.g., adding -DSCALAR to make.type.hydro, and then make TYPE=hydro HOST=lux -j 20 fails to compile).

I can't log into the jenkins/pr-merge link to see though.

evaneschneider commented 5 days ago

I'm actually not sure what's going on - something seems to be going wrong with our clang-tidy setup on the cluster, we haven't been able to run it successfully on any builds this week. All of the builds and tests for this commit passed though, so I'm just going to merge it while I try to figure out what's happening.