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

Corrections on the N-S code #97

Merged
merged 10 commits into from Dec 23, 2022

Conversation

QLRO
Copy link
Contributor

@QLRO QLRO commented Sep 9, 2022

Dear Dokken,

Thank you for writing this excellent tutorial!

I would like to point out a few minor mistakes in your implementation. In particular, density rho is missing in several places. While this does not affect the results in your example since you have set rho=1, it may cause problems in general case.

I made the quick edits using github web interface. The same issue exists in the other N-S example too (also the theory part and the py files). I apologize for not having enough time to change them all locally.

P. S. For the DFG 2D-3 benchmark, I have not been not able to reproduce the entire 8s results by either simply increasing to 12800 steps over 8s or with lower dt (currently trying 1/3200). Have you been able to reproduce the entire 8s results? If yes, what settings did you use?

Best,

@QLRO
Copy link
Contributor Author

QLRO commented Sep 9, 2022

I have some free time this weekend. I am happy to help you make the edits if it is ok.

@jorgensd
Copy link
Owner

jorgensd commented Sep 9, 2022

@QLRO I observe that there are some stability issues after ~3.7 seconds. This is probably down to the mesh being to coarse (as the mesh depends on gmsh and its version, it might have slightly changed in the last two years). I will push a fix for this.

Some general comments to your PR:

  1. Make sure that it is consistent everywhere.
  2. Use jupytext to also update the .py file in the repo (this should happen automatically).

@jorgensd jorgensd self-requested a review September 9, 2022 12:33
@QLRO
Copy link
Contributor Author

QLRO commented Sep 10, 2022

I think that's everything I can find. Please have a look at the changes and let me know if I missed anything. Thanks!

@jorgensd
Copy link
Owner

Stability of the method has been improved in:
#99

@QLRO
Copy link
Contributor Author

QLRO commented Sep 10, 2022

Dear Dokken,

Great! Thank you for your hard work!

I have updated both the notebook and the py file of the benchmark example to your improved code. Please see if there are any issues.

Best,

@jorgensd jorgensd merged commit d2cebba into jorgensd:dokken/jupyterbook Dec 23, 2022
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.

None yet

2 participants