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

Add 3D advection-diffusion #57

Merged
merged 9 commits into from
Jul 2, 2022

Conversation

jbisits
Copy link
Collaborator

@jbisits jbisits commented Jul 1, 2022

This PR adds functionality to advect-diffuse a passive tracer in a 3D domain. It adds a new method to TracerAdvectionDiffusion.Problem that takes in a flow of type ThreeDAdvectingFlow. The setup of the Problem is the same as for the one and two dimensional methods already there.

I have written some tests but have not written an example. With the setup and simulation being so similar to the 1D and 2D cases do you think it necessary to have an example for the 3D advection-diffusion? Happy to write one just thought would see what you thought first.

I also have not updated any documentation files yet (e.g. the manual) but can do so if you think everything looks to be performing ok.

All seems to be working except the test for a time varying flow in 3D fails. Not sure why yet.
Had a wrong variable in `calc_N!`. Fixed now all tests pass. Need to do some beautifications so that indenting is the same.
@jbisits jbisits requested a review from navidcy July 1, 2022 12:15
@navidcy
Copy link
Member

navidcy commented Jul 2, 2022

@jbisits I thought we change the diffusivities so that κ is x. It's a bit more intuitive now that we have 1D, 2D, 3D.
Have a look what I did.

@navidcy
Copy link
Member

navidcy commented Jul 2, 2022

test_timedependentvel2D and test_timedependentvel3D fail... Hm...

Copy link
Member

@navidcy navidcy left a comment

Choose a reason for hiding this comment

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

Nice work. Let's figure out what's happening with the tests before we merge.

@navidcy
Copy link
Member

navidcy commented Jul 2, 2022

I don't think an example is needed.

@navidcy navidcy added the enhancement New feature or request label Jul 2, 2022
@navidcy navidcy linked an issue Jul 2, 2022 that may be closed by this pull request
@navidcy
Copy link
Member

navidcy commented Jul 2, 2022

OK, fixed the tests! I had broken the calcN! for 2D/3D... :)

@navidcy navidcy merged commit aebc084 into FourierFlows:main Jul 2, 2022
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.

Generalize TracerAdvDiff module to 1D and 3D
2 participants