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

Each tracer should have its own diffusivity #430

Closed
ali-ramadhan opened this issue Sep 27, 2019 · 3 comments · Fixed by #452
Closed

Each tracer should have its own diffusivity #430

ali-ramadhan opened this issue Sep 27, 2019 · 3 comments · Fixed by #452
Labels
abstractions 🎨 Whatever that means bug 🐞 Even a perfect program still has bugs
Milestone

Comments

@ali-ramadhan
Copy link
Member

Not an immediate concern and but right now the diffusivity κ is shared between temperature and salinity whereas they should be different.

Perhaps this should be worked on at the same time as arbitrary tracer fields (#25).

@ali-ramadhan ali-ramadhan added bug 🐞 Even a perfect program still has bugs abstractions 🎨 Whatever that means labels Sep 27, 2019
@ali-ramadhan ali-ramadhan added this to the v1.0 milestone Sep 27, 2019
@glwagner
Copy link
Member

I agree that this is naturally addressed when arbitrary tracer fields are implemented.

Note that this applies not just to diffusivities (if you're referring to ConstantIsotropicDiffusivity), but to background diffusivities associated with LES closures, and also to the turbulent prandtl numbers associated with the Smagorinsky closures.

@glwagner
Copy link
Member

My plan:

  • When the user specifies a single diffusivity, this diffusivity is applied to all tracer fields
  • Otherwise, diffusivities are specified by a named tuple, where names correspond to diffusivity names, eg
tracers = (:T, :S)

closure = ConstantIsotropicDiffusivity=1e-6, κ=(T=1e-6, S=1e-7))

and

tracers = (:T, :S)

closure = ConstantIsotropicDiffusivity=1e-6, κ=1e-6, tracers=tracers)

would be possible use patterns. The tracers keyword is only required when the diffusivity is not itself a named tuple.

Or, possibly we can require that tracers is supplied, eg

closure = ConstantIsotropicDiffusivity(tracers, ν=1e-6, κ=(T=1e-6, S=1e-7))

This last pattern would be useful if, for example, we want to do input validation on the diffusivity tuple.

We may also want to think about designs / syntactic sugar for making the handling of large numbers of passive tracers easier.

@glwagner glwagner mentioned this issue Oct 9, 2019
1 task
@ali-ramadhan
Copy link
Member Author

Awesome! Yeah just reviewed PR #452 which looks great. I just opened the issue to keep track of things, but the arbitrary tracers thing got resolved pretty quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abstractions 🎨 Whatever that means bug 🐞 Even a perfect program still has bugs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants