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

Feature: Transporting passive scalars #465

Open
ThomasHowarth opened this issue Jan 10, 2025 · 5 comments · May be fixed by #467
Open

Feature: Transporting passive scalars #465

ThomasHowarth opened this issue Jan 10, 2025 · 5 comments · May be fixed by #467
Labels
enhancement New feature or request

Comments

@ThomasHowarth
Copy link
Contributor

ThomasHowarth commented Jan 10, 2025

I'm working on adding in passive scalar transport, and I was wondering if anyone had any thoughts on whether the number of transported scalars should be specified at compile time or runtime. Some existing things are floating about to get started on this (e.g. m_nAux parameter, auxiliaries MultiFAB etc.), but there is also mention of implementing an NAUX variable in the index file.

@ThomasHowarth ThomasHowarth added the enhancement New feature or request label Jan 10, 2025
@dmontgomeryNREL
Copy link
Contributor

dmontgomeryNREL commented Jan 10, 2025

Hi @ThomasHowarth, this would be a very welcomed addition! I too was confused by the existence of m_nAux when I added the ODEQty's #427. I think for consistency that NUM_AUX should be specified in the GNUmakefile similar to NUM_ODE, NUMSOOTVAR, etc. Since m_nAux is already defined and passed into multiple functions, it might be easiest to set m_nAux = NUM_AUX here in PeleLMeX.H. Not an elegant solution, but at least it's consistent with the other variables. Any thoughts @baperry2?

@dmontgomeryNREL
Copy link
Contributor

Another thought is that m_nAux should really be m_nAdv for consistency with PeleC. Considering that m_nAux has always been zero, this change shouldn't impact any users but it would require a lot of searching and renaming. Ideally we'd rename the ODEQty's to aux quantities and have passive scalars as advected quantities.

@baperry2
Copy link
Collaborator

baperry2 commented Jan 10, 2025

Passive scalar transport in PeleLMeX would definitely be a good feature to get into the main code.

As you mention it looks like two paths for this have been started and abandoned - using m_nAux and the auxiliaries multifab or integrating passive scalars into the current state multifab using AUX in the Index file. In principal, the former is attractive because it could allow for the number of passive scalars to be changed at runtime rather than compile time, but implementation would be more complex. An advantage of lumping passive scalars in with the rest of the state is that we'd have access to them whenever we have access to the state, so function interfaces wouldn't need to be modified for the most part. @esclapez - any history or considerations I'm missing here?

When @dmontgomeryNREL implemented the ODEQty capability (scalars that are not transported), we went with the compile time approach due to easier implementation and that being closer to the PeleC implementation. I know @bssoriano also has a branch of the code somewhere with mixture fraction and fluid age transport through a compile time option similar to AUX in the index file.

@esclapez
Copy link
Collaborator

@baperry2 the initial plan (when starting LMeX) was indeed to follow PeleC and use a compile-time number of scalars within the "regular" state MF. It wasn't a priority initially, so I didn't push it.
I think the compile-time indices might grow out of control as more capabilities are added to the solver, so I would be in favor of trying to define those at runtime for more readability and flexibility on the user side. A bit more involved to implement (function interface changes) and we might want to bundle all the "aux" into a single MF (efield, soot, ODEQty, ...) and keep state 'clean'. I'm wondering how much of a penalty we would take if we decided to use runtime for all containers (drop the index stuff entirely). That would bring us a step closer to a less compilation in Pele.

@baperry2
Copy link
Collaborator

Thanks Lucas! I agree combining all the aux stuff into a single runtime determined multifab is probably the ideal strategy - I don't think there'd be much of a performance hit for those and it would make things more user friendly and maintainable.

@ThomasHowarth - if you want to get that started by setting up the separate multifab for passive scalar transport with scalars determined at runtime that would be awesome (we could fold the other stuff like Soot and ODEQty into it later). But up to you whether you want to pursue runtime or compile time implementation.

@ThomasHowarth ThomasHowarth linked a pull request Jan 17, 2025 that will close this issue
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 a pull request may close this issue.

4 participants