Port mixing length model to the gpu path (and gradient fix, part 2) #226
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The goal is to enable MixingLengthTransport on the gpu. Prior to this commit, this class was surrounded by a #if that prevented it from being compiled in gpu builds. Simply removing this #if leads to test failures, b/c the design of the MixingLengthTransport---specifically, where some methods call the same method from a different class. This seems to confuse nvcc, even though the MixingLengthTransport class is not even instantiated in the gpu path yet. Specifically, test failures are observed (e.g., the parallel tests in cyl3d.gpu.test fail). I think the problem is related to the issue first noticed in #184 and then dealt with (via a workaround) in #203. To work around it here, a new class MolecularTransport is introduced, where the flux and source transport functions can have a different signature (b/c they never need the distance function, for example). This is enough to avoid the problem---i.e., all the tests pass.
Wanted to use dynamic_cast to cast the TransportProperties pointer that M2ulPhyS holds to MolecularTransport (as in the cpu path), but host-side dynamic_cast on a device-side pointer seg faults, and device-side call to dynamic_cast isn't allowed. So... go ahead and pass the TransportProperties pointer and use a regular cast on the device.
Two bugs in setting the distance function arrays for the interior faces on the gpu path. This data are only used in the MixingLengthTransport class, and since we only just started trying to make that work on the gpu path, it was just noticed now. The useBCinGrad flag is not implemented on the gpu yet, so pipe.mix.test fails. However, if that flag is set to false on the cpu path, then the result of that case is the same on the gpu and the cpu (to within the soln_differ tolerance).
This test will fail currently, b/c the useBCinGrad option is not implemented for the gpu path. Goal now is to fix this.
This commit implements the useBCinGrad flag on the gpu path. When this flag is on, for a no-slip adiabatic wall, the velocity (0) and user-set temperature are used in boundary terms that appear in the system defining the gradient. Otherwise, these terms are ignored, which is incorrect. Since the terms were ignored before, there was no infrastructure for looping over boundary faces in forming the gradient system on the gpu path. This commit adds this infrastructure, which is slightly more complicated than analogous face loops (i.e., interior and 'shared' faces) due to the fact that periodic faces are counted as boundary faces by mfem (see mfem::Mesh::GetNBE vs mfem::Mesh::GetNFbyType). The infrastructure is added in the minimal way possible---i.e., just for no-slip, isothermal walls. The BC infrastructure will need to be refactored to make it easy to support all BC types.
In addition to modifying the gradient calculation, when the useBCinGrad flag is on, we use a mass-conserving variant of the weak wall BC. This commit implements this on the gpu path.
Two mods... first, add logic to determine how to run parallel job to pipe.mix.test, so that it can run with flux on systems where mpirun isn't available. Second, relax pipe.mix.test tolerance slightly. Observed small increase in deltas on some (not all) cuda systems where tests run.
During development, BC info was hard-coded (to 3-velocity no slip, 300K). This commit removes this. Velocity is still assumed to be zero, but temperature is taken from the input file. This is hacked in via the boundaryFaceIntegrationData struct, which is suboptimal and should be refactored to easily support other BC types.
trevilo
force-pushed
the
dev-mixing-length-gpu
branch
from
144f281
to
d536b9b
Compare
trevilo
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds the mixing length turbulence model to the gpu path. Two notable developments are included:
nvcc(presumably related to issues observed on CUDA nvlink warning and memory error with 'virtual' specifier #184 and discussed further in PR Port axisymmetric solver to gpu #203) a new classMolecularTransporthas been added in order to distinguish the molecular transport calculations from the mixing length transport calculations (which call the molecular transport).useBCinGradflag has been implemented on the gpu path, which is important b/c this flag corrects the gradient calculation to account for boundary data (see Boundary condition contribution to gradient system #198 and PR Toward boundary gradient fix: part 1 #216). We still only support isothermal walls BCs with this flag.As evidence that the implemenation is correct,
pipe.mix.testhas been added to the tests that run on gpu systems.