Skip to content
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.

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

Hyperbolic Adv-Diff, Navier-Stokes 1D/2D #811

Draft
wants to merge 41 commits into
base: main
Choose a base branch
from
Draft

Hyperbolic Adv-Diff, Navier-Stokes 1D/2D #811

wants to merge 41 commits into from

Conversation

MagLuc
Copy link

@MagLuc MagLuc commented Aug 21, 2021

In this fork the Advection-Diffusion and Navier-Stokes equations in one and two dimensions are added to Trixi. Both equations are implemented with hyperbolic formulations, which were developed by Hiroaki Nishikawa. Serveral examples to all the new equations are also added.

MagLuc added 30 commits July 20, 2021 17:18
Files for the 1d and 2d Advection-Diffusion equation and files for the 1d and 2d Navier-Stokes equations are included.
The functions to call the Advection-Diffusion equation and the Navier Stokes equations need to be exported in the Trixi.jl file.
In case of the 1d hyperbolic Navier-Stokes equations the value of the "du" variable needs to be multiplied by a precondition matrix. This multiplication is added to the "rhs!" function.
Is case of the 2d Navier-Stokes equations the value of the "du" variable needs to be multiplied by a precondition matrix. This multiplication is added to the "rhs!" function.
The function is necessary for the Navier-Stokes precondition matrix multiplication.
An initial guess is added to the example exp_nonperiodic
The added elixirs execute the changed or added examples for the hyperbolic Navier-Stokes equations
Exporting the new examples for the Advection-Diffusion and the Navier-Stokes equations. The function calc_viscous_shock_solution which runs a fortran function to calculate an initial solution is also exported.
Copy link
Member

@andrewwinters5000 andrewwinters5000 left a comment

Choose a reason for hiding this comment

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

Overall very nice work @MagLuc ! This is a very nice set of interesting equation systems to add into the Trixi capabilities.

I have put together a first batch of comments and suggestions. My initial reaction is that you have constructed a lot of manufactured solution testcases. We should strive to remove unnecessary ones and "trim" it down to effectively test your new equation systems but avoid duplication within the elixirs. You should also try to move the initial_condition + source_term functions into the elixir (when possible) as motivated by #685 . Once we have settled on which tests to keep you should then add them to the automatic test scripts to make sure that we keep the code coverage.

We should also get some feedback from @gregorgassner , @sloede and @ranocha about the way to move forward with this.


"""
Example "idlCFD"
This example is taken from the free book "I Do Like CFD".
Copy link
Member

Choose a reason for hiding this comment

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

Can you please be more specific here. What page number / equation number is it. I think it is from Section 7.3.1 but it would be good to update the docstring here

periodicity=false) # set maximum capacity of tree data structure


initial_condition = initial_condition_idlCFD_nonperiodic
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
initial_condition = initial_condition_idlCFD_nonperiodic
initial_condition = initial_condition_boundary_layer

I do not think that this name is appropriate. We should change it to something more descriptive/obvious for other users. Like including the phrase "boundary layer", then it is implicit that it is not a periodic testcase

periodicity=false) # set maximum capacity of tree data structure


initial_condition = initial_condition_myexp_nonperiodic
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
initial_condition = initial_condition_myexp_nonperiodic
initial_condition = initial_condition_convergence_nonperiodic

Here the name of this initial condition is also vague. I think it means that it is a manufactured solution involving exponentials but it should be adjusted

###############################################################################
# ODE solvers, callbacks etc.

# Create ODE problem with time span from 0.0 to 1.0
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Create ODE problem with time span from 0.0 to 1.0
# Create ODE problem with time span from 0.0 to 3.0

periodicity=false) # set maximum capacity of tree data structure


initial_condition = initial_condition_mytest_nonperiodic
Copy link
Member

Choose a reason for hiding this comment

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

what does mytest mean in this context? What does this configuration test that elixir_hypadvectiondiff_myexp_nonperiodic.jl and elixir_hypadvectiondiff_idlCFD_nonperiodic.jldo not?

If possible we could maybe remove this testcase

src/equations/hyperbolic_navier_stokes_2d.jl Outdated Show resolved Hide resolved
src/equations/hyperbolic_navier_stokes_2d.jl Outdated Show resolved Hide resolved
Comment on lines 472 to 473
# λ_max_ll = (abs(v1_ll)+abs(v2_ll))/2 + sqrt(T_ll) + abs(mu_h_ll / (u1_ll * L))
# λ_max_rr = (abs(v1_rr)+abs(v2_rr))/2 + sqrt(T_rr) + abs(mu_h_rr / (u1_rr * L))
Copy link
Member

Choose a reason for hiding this comment

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

Remove?

mu_v = 4/3 * mu # viscosity of stress
mu_h = gamma*mu/Pr # viscosity of heat flux

# λ_max = (abs(v1)+abs(v2))/2 + sqrt(T) + abs(mu_h / (rho * L))
Copy link
Member

Choose a reason for hiding this comment

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

Remove?

src/equations/hyperbolic_navier_stokes_2d.jl Outdated Show resolved Hide resolved
@DanielDoehring DanielDoehring changed the title Mag luc patch 1 Hyperbolic Adv-Diff, Navier-Stokes 1D/2D Apr 9, 2024
@DanielDoehring DanielDoehring added the enhancement New feature or request label Apr 10, 2024
@DanielDoehring DanielDoehring marked this pull request as draft April 24, 2024 15:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants