Skip to content

Comments NSF (Entropy-Var) Gradients #2528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+15 −15
Draft

Comments NSF (Entropy-Var) Gradients #2528

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4fc46a5
Comments NSF (Entropy-Var) Gradients
DanielDoehring Aug 22, 2025
91ce9ac
enhance
DanielDoehring Aug 22, 2025
4b0fe3d
update
DanielDoehring Aug 22, 2025
8818065
Merge branch 'main' into NSF_EntropyVarLabeling
DanielDoehring Aug 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/equations/compressible_navier_stokes_1d.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ end

# TODO: parabolic
# This is the flexibility a user should have to select the different gradient variable types
# varnames(::typeof(cons2prim) , ::CompressibleNavierStokesDiffusion1D) = ("v1", "v2", "T")
# varnames(::typeof(cons2entropy), ::CompressibleNavierStokesDiffusion1D) = ("w2", "w3", "w4")
# varnames(::typeof(cons2prim) , ::CompressibleNavierStokesDiffusion1D) = ("rho", "v1", "T")
# varnames(::typeof(cons2entropy), ::CompressibleNavierStokesDiffusion1D) = ("w1", "w2", "w3")
Comment on lines +127 to +128
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am still not a fan of this. I my eyes we should have a separate set of variables of size nvars_grad to avoid the computation of unnecessary gradients like d_rho or d_w1. This commented section of code was intended to reflect such a desire. So changing it to include the unnecessary gradient values goes against its spirit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, this would be nice - I gave it a quick look this morning but this is a major project, as a whole set of functions need to be rewritten.

For now, however, I would like to have a comment reflecting what is, and maybe add a comment for this TODO. We still have it here: #1147


function varnames(variable_mapping,
equations_parabolic::CompressibleNavierStokesDiffusion1D)
Expand Down Expand Up @@ -215,10 +215,10 @@ end
return cons2prim(entropy2cons(u_transformed, equations), equations)
end

# Takes the solution values `u` and gradient of the entropy variables (w_2, w_3, w_4) and
# reverse engineers the gradients to be terms of the primitive variables (v1, v2, T).
# Takes the solution values `u` and gradient of the entropy variables w and
# reverse engineers the gradients to be terms of the primitive variables u_prim = (rho, v1, v2, T).
Comment on lines +218 to +219
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here. I would like to avoid unnecessary gradients as it wastes computation time and memory. Although, I understand that having a gradient vector of variables that is one less than the conservative variable vector could be an annoying restructuring of the existing code.

# Helpful because then the diffusive fluxes have the same form as on paper.
# Note, the first component of `gradient_entropy_vars` contains gradient(rho) which is unused.
# Note, the first component of `gradient_entropy_vars` w1 contains gradient(rho) which is unused.
# TODO: parabolic; entropy stable viscous terms
@inline function convert_derivative_to_primitive(u, gradient,
::CompressibleNavierStokesDiffusion1D{GradientVariablesPrimitive})
Expand All @@ -230,7 +230,7 @@ end
equations::CompressibleNavierStokesDiffusion1D{GradientVariablesEntropy})

# TODO: parabolic. This is inefficient to pass in transformed variables but then transform them back.
# We can fix this if we directly compute v1, v2, T from the entropy variables
# We can fix this if we directly compute v1, T from the entropy variables
u = entropy2cons(w, equations) # calls a "modified" entropy2cons defined for CompressibleNavierStokesDiffusion1D
rho, rho_v1, _ = u

Expand Down
10 changes: 5 additions & 5 deletions src/equations/compressible_navier_stokes_2d.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ end

# TODO: parabolic
# This is the flexibility a user should have to select the different gradient variable types
# varnames(::typeof(cons2prim) , ::CompressibleNavierStokesDiffusion2D) = ("v1", "v2", "T")
# varnames(::typeof(cons2entropy), ::CompressibleNavierStokesDiffusion2D) = ("w2", "w3", "w4")
# varnames(::typeof(cons2prim) , ::CompressibleNavierStokesDiffusion2D) = ("rho", "v1", "v2", "T")
# varnames(::typeof(cons2entropy), ::CompressibleNavierStokesDiffusion2D) = ("w1", "w2", "w3", "w4")
Comment on lines +127 to +128
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar here. Changing this commented code goes against the original intent in its creation. We discussed several years ago to mimic the computation of the gradients to match FLUXO. Perhaps we are willing to eat the computational overhead of computing unused gradients, but this is something we should discuss as a group.


function varnames(variable_mapping,
equations_parabolic::CompressibleNavierStokesDiffusion2D)
Expand Down Expand Up @@ -237,10 +237,10 @@ end
return cons2prim(entropy2cons(u_transformed, equations), equations)
end

# Takes the solution values `u` and gradient of the entropy variables (w_2, w_3, w_4) and
# reverse engineers the gradients to be terms of the primitive variables (v1, v2, T).
# Takes the solution values `u` and gradient of the entropy variables w and
# reverse engineers the gradients to be terms of the primitive variables u_prim = (rho, v1, v2, T).
# Helpful because then the diffusive fluxes have the same form as on paper.
# Note, the first component of `gradient_entropy_vars` contains gradient(rho) which is unused.
# Note, the first component of `gradient_entropy_vars` w1 contains gradient(rho) which is unused.
# TODO: parabolic; entropy stable viscous terms
@inline function convert_derivative_to_primitive(u, gradient,
::CompressibleNavierStokesDiffusion2D{GradientVariablesPrimitive})
Expand Down
10 changes: 5 additions & 5 deletions src/equations/compressible_navier_stokes_3d.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ end

# TODO: parabolic
# This is the flexibility a user should have to select the different gradient variable types
# varnames(::typeof(cons2prim) , ::CompressibleNavierStokesDiffusion3D) = ("v1", "v2", "v3", "T")
# varnames(::typeof(cons2entropy), ::CompressibleNavierStokesDiffusion3D) = ("w2", "w3", "w4", "w5")
# varnames(::typeof(cons2prim) , ::CompressibleNavierStokesDiffusion3D) = ("rho", "v1", "v2", "v3", "T")
# varnames(::typeof(cons2entropy), ::CompressibleNavierStokesDiffusion3D) = ("w1", "w2", "w3", "w4", "w5")
Comment on lines +127 to +128
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same in 3D.


function varnames(variable_mapping,
equations_parabolic::CompressibleNavierStokesDiffusion3D)
Expand Down Expand Up @@ -263,10 +263,10 @@ end
return cons2prim(entropy2cons(u_transformed, equations), equations)
end

# Takes the solution values `u` and gradient of the entropy variables (w_2, w_3, w_4, w_5) and
# reverse engineers the gradients to be terms of the primitive variables (v1, v2, v3, T).
# Takes the solution values `u` and gradient of the entropy variables w and
# reverse engineers the gradients to be terms of the primitive variables u_prim = (rho, v1, v2, v3, T).
# Helpful because then the diffusive fluxes have the same form as on paper.
# Note, the first component of `gradient_entropy_vars` contains gradient(rho) which is unused.
# Note, the first component of `gradient_entropy_vars` w1 contains gradient(rho) which is unused.
# TODO: parabolic; entropy stable viscous terms
@inline function convert_derivative_to_primitive(u, gradient,
::CompressibleNavierStokesDiffusion3D{GradientVariablesPrimitive})
Expand Down
Loading