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

[Bug]: Moment transforms require many methods from deprecated Lattice #222

Closed
PhiSpel opened this issue Aug 20, 2024 · 0 comments · Fixed by #241
Closed

[Bug]: Moment transforms require many methods from deprecated Lattice #222

PhiSpel opened this issue Aug 20, 2024 · 0 comments · Fixed by #241
Assignees
Labels
bug Something isn't working

Comments

@PhiSpel
Copy link
Contributor

PhiSpel commented Aug 20, 2024

What happened?

I tried implementing the test_moments functions for #172
The Transform classes require Lattice in init, partially for the Context information, partially for functions such as mv, sometimes for q (in base class, which could be replaced!), D2Q9Lallemand even uses the equilibrium, which is now in Flow. We should discuss how to harmonize this.

Further context

This is strongly related to #169

Suggestion

Often, the functions could be made independent of flow: The observables rho, u, velocity, etc., are not redefined in Flow subclasses.
However, some calculations require information, e.g., about the stencil. This could be solved by always passing the flow itself to the calculation. This would not be very different from the current setup, where the flow is just self. The issue here is that functions which do not numerically depend on a simulation domain are made depend on it in implementation.

So, I suggest moving the functions into something like the util directory and importing them when they are needed, passing the required arguments such as f, rho, equilibrium, etc.
To additionally reduce the number of methods, most einsum usages (see #111 ) such as mv (matrix-vector-product) or j (momentum) should be called inline as they have hardly any usages outside of flow.

@PhiSpel PhiSpel added the bug Something isn't working label Aug 20, 2024
@McBs McBs closed this as completed in #241 Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants