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

Describe Navier-Stokes Diffeq example mesh by gmsh api usage and translate via FerriteGmsh #498

Merged
merged 37 commits into from
Jun 11, 2024

Conversation

koehlerson
Copy link
Member

Need to include an optional finer discretization with vortex street. Currently only coarse discretization for CI is in the literate file. Will test around when I'm back at my workstation

@codecov-commenter
Copy link

codecov-commenter commented Oct 2, 2022

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.75%. Comparing base (a10097a) to head (c7bd22e).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #498   +/-   ##
=======================================
  Coverage   93.75%   93.75%           
=======================================
  Files          36       36           
  Lines        5441     5441           
=======================================
  Hits         5101     5101           
  Misses        340      340           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@termi-official termi-official left a comment

Choose a reason for hiding this comment

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

This might work.

docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
@koehlerson
Copy link
Member Author

@fredrikekre mesh from stokes example chokes 🤔

@koehlerson koehlerson force-pushed the mk/nsgmsh branch 4 times, most recently from aaefd4b to a557759 Compare October 4, 2022 11:05
@koehlerson
Copy link
Member Author

I could not reproduce the vortex street with the updated mesh, but it's very likely not a problem with the mesh itself

@termi-official
Copy link
Member

To elaborate on the likely issue: The solution for the formulation is not unique up to a constant value in the pressure. For the currently used mesh with equisized elements the linear solvers seems to not have trouble finding a close sequence of constants. We should fix this with the trick from the stokes example, where we force the constant to a proper value by setting the boundary integral of the pressure to zero. Does not seem to be super straight forward in this example.

@termi-official
Copy link
Member

Can we make the grid generation deterministic?

@termi-official
Copy link
Member

I could find several open issues.

  1. the pressure ramp protocol causes discontinuities in the second derivative. The chosen solver uses a second derivative approximation to control the time step. This causes the failure at the end of the ramp.
  2. Solver tolerances were too high for the unstructured grid to resolve the oscillations sufficiently.
  3. Since we cannot enforce the time-dependent constraints consistently everywhere in OrdinaryDiffEq (interface does not allow this yet) the time step length degrades severly for the adaptive implicit Euler. Higher order methods just fail.
  4. The evaluation of the right hand side is dead slow. After merging this we might want to open 2 issues here: One about speeding up the solver via custom solver and one about parallelizing the rhs evaluation (+user-defined, parallel Jacobian). The former should also be a first good step towards enforcing time-dependent constraints.
  5. Internal norms were erroneous as they included the constraints.

I will push the fixes for 1,2,5 later today, after commenting everything. This should then make the PR ready to merge.

@termi-official
Copy link
Member

Also as a future project idea: 3D Navier Stokes with MPI parallelization in tandem with OrdinaryDiffEq.

Copy link
Member

@termi-official termi-official left a comment

Choose a reason for hiding this comment

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

Some final changes and this should be good to go.

docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
docs/src/literate/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
@termi-official
Copy link
Member

Should be ready transiently through #917 .

termi-official and others added 4 commits May 27, 2024 08:06
* Tryi Fixi

* Dep

* Reproducer.

* Manifest.

* Finally fix the Navier-Stokes solver. At least partially.

* Dump other examples.

* fix tag numbers of rectangle curves from occ

* Fix example and recover vortex street.

* Revert

* Revert manifest

* Remove trailing message.

* Update docs/src/literate-tutorials/ns_vs_diffeq.jl

Co-authored-by: Maximilian Köhler <[email protected]>

* Apply suggestions from code review

Co-authored-by: Maximilian Köhler <[email protected]>

* Format.

* Tweaks

* Fixed. Now we just need to wait for the next OrdinaryDiffEq release.

* Add new DiffEq release.

---------

Co-authored-by: Maximilian Köhler <[email protected]>
Co-authored-by: termi-official <[email protected]>
@termi-official
Copy link
Member

xref SciML/OrdinaryDiffEq.jl#2238 the information needs to be added

Copy link
Member

@termi-official termi-official left a comment

Choose a reason for hiding this comment

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

Thanks Maxi! I will update the example to use the TimeChoiceIterator in a separate PR later after the interpolation issue in DifferentialEquations.jl is resolved.

@termi-official
Copy link
Member

Last commit broke the gif :D

Copy link
Member

@termi-official termi-official left a comment

Choose a reason for hiding this comment

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

LGTM otherwise.

docs/src/literate-tutorials/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
docs/src/literate-tutorials/ns_vs_diffeq.jl Outdated Show resolved Hide resolved
@koehlerson koehlerson merged commit bdd5b41 into master Jun 11, 2024
11 checks passed
@koehlerson koehlerson deleted the mk/nsgmsh branch June 11, 2024 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants