search

quinoacomputing / quinoa

Adaptive computational fluid dynamics
https://quinoacomputing.github.io
Other
101 stars 21 forks source link

Remove equation systems #591

Closed jbakosi closed 1 year ago

jbakosi commented 1 year ago

Note: targeting branch 'system1', which should be merged first.

This touches a lot of, but mostly straightforward, code.

This removes systems. Let me know if I missed something.

Another potential, hopefully final step, is to remove systems from the parser grammar, but the vectors originally intended for systems can now be repurposed for solvers operating on different meshes. Let's discuss any pros and cons of this and/or whether we should simply reuse the existing vectors in the inputdeck parser grammar or redesign that a bit. My vote is to use it as is for now, and a better use of time is to replace PEGTL with LUA eventually.

This change is Reviewable

jbakosi commented 1 year ago

It just came into my mind: Shouldn't we keep all this for passing down the meshid instead?

jbakosi commented 1 year ago

"All this" meaning basically canceling this PR. No? We could rename 'system' to meshid but that's it. Otherwise, how will we pass down the meshid to all these functions? I think that will be necessary soon for m2m problems, no? We could reintroduce that with a last function arg defaulting to 0. Will that not be needed for pretty much all code this PR touches?

jbakosi commented 1 year ago

It is true that that's the design and that would be true on the surface of storing and passing this data down to the operators, but look at the diff of this PR and see what that system arg has been used for: mostly to look up values from inputdeck of which there is a single copy on each PE, storing global config data for all meshes. Surely at the lowest levels the different solvers will need to know what mesh they operate on. No?

jbakosi commented 1 year ago

I'm talking about possibly keeping the vectors in the inputdeck but changing the semantics of what it contains, i.e., not for system index but for meshid. Would that be useful in the immediate future? At first I thought it might be useful, but I'm now struggling to find a use-case so it is probably not worth keeping it.

For example, I thought about a need to look up a different way of setting user-defined ICs dependent on which mesh/solver we are initializing, but that does not make much sense. Another example might be when needing to use a different material, e.g., gamma, depending on which solver (mesh) is being solved. But that also does not make sense because the fluid solved with all meshes will be the same. Will it always be? (I'm considering the simplest initial use-case of coupling here with rigid bodies embedded in a background flow where the flow around each object is the same fluid.) Basically, I'm wondering if it makes sense to plan for a case where anything that is needed from the inputdeck would depend on the meshid during a run. Since the inputdeck is meshid-agnostic (it has data on all meshids), it is the same copy on all PEs and for all solvers.

But I think it is probably the best to eliminate this, which simplifies code, and reintroduce whatever is needed when/if needed.

jbakosi commented 1 year ago

Okay, thanks for the explanation. I see no point in not merging this PR then. As I mentioned in the intro, I will take a 3rd pass removing the vectors from the input deck. Please go ahead and merge system1 and system2 if/when they pass CI.