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

Lagrangian Smagorinsky for 2D flows #4009

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

simone-silvestri
Copy link
Collaborator

At the moment, Lagrangian Smagorinsky does not work for 2D flows because nodes on flat topologies return nothing.
It is a very easy correction to make it work.

However, I know that Lagrangian Smagorinsky is not really meant for 2D flows, so if we want to keep this incompatibility to prevent people from using it, it is also fine (I vote for correcting it anyways).

@tomchor please advise

@tomchor
Copy link
Collaborator

tomchor commented Dec 18, 2024

I think we should keep it incompatible with anything but 3D flows. Not only it goes against LES assumptions, but it just doesn't work and the results look terrible (because they violate said assumptions). Personally, I don't see anything to gain from generalizing it.

@simone-silvestri
Copy link
Collaborator Author

simone-silvestri commented Dec 18, 2024

I would propose to fix the error and including a warning (or error) when the topology is Flat.
Users will try using it on flat domains regardless, so it is better to have an explicit warning (or error) instead of an obscure error deep in the weed of the kernel.

@tomchor
Copy link
Collaborator

tomchor commented Dec 18, 2024

I would propose to fix the error and including a warning (or error) when the topology is Flat. Users will try using it on flat domains regardless, so it is better to have an explicit warning (or error) instead of an obscure error deep in the weed of the kernel.

Agreed. Should we do the same for other LES closures?

@simone-silvestri
Copy link
Collaborator Author

why not, what should we warn against? Probably AnisotropicMinimumDissipation() for 2D domains as well? I Think TwoDimensionalLeith is fine for both 2d and 2d domains

@glwagner
Copy link
Member

I personally don't really agree with the idea of having "hidden bugs" because of our personal theory about how a closure should work. So I would say this should work in 2D. Otherwise if someone wants to use this code to experiment with different formulations that actually do work in 2D, they will have a mountain to climb for no reason other than we are not friendly.

@glwagner
Copy link
Member

why not, what should we warn against? Probably AnisotropicMinimumDissipation() for 2D domains as well? I Think TwoDimensionalLeith is fine for both 2d and 2d domains

We have historically been against trying to tell users our opinion about their physics. I don't think we should adopt a new policy now...

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.

3 participants