Fixed the momentum exchange method in the JAX backend. #101
Conversation
| @@ -53,9 +53,6 @@ def __init__( | |||
| mesh_vertices, | |||
| ) | |||
|
|
|||
| # Set the flag for auxilary data recovery | |||
There was a problem hiding this comment.
was not needed for ExtrapolationOutflow BC. that BC directly uses _f_post_collision to put all the necessary ingredients.
There was a problem hiding this comment.
I am not sure. We created reconstruction originally for this BC!:
# Update the auxilary data for this BC using the neighbour's populations stored in f_aux and
# f_pre (post-streaming values of the current voxel). We use directions that leave the domain
# for storing this prepared data.
If you don[t recover, you'll 100% overwrite this data...
There was a problem hiding this comment.
I have changed those comments since then. I can remove this part and add it to another PR if that helps you understand why but this BC assembles all the information and stores it directly in _f_post_collision which is then recovered when we write in f_1 (not f_0 as in bc_recovery). Just like how halfway bc uses post-collision values in the opposite direction taken from f_0, this BC also takes "post-collision" values taken from f_0 which are hand crafted earlier inside a function that is called "update_bc_auxilary_data".
There was a problem hiding this comment.
I have to think about this more to make sure it is correct. We're rushing PR merges for no reason and we're reverting them back or fix them almost immedately a few days later. I'll check this next week.
There was a problem hiding this comment.
We are not reverting anything! There are minor things that have been missed in the reivews or unknown new bugs that are discovered.
| @@ -16,7 +16,7 @@ class D3Q19(VelocitySet): | |||
|
|
|||
| def __init__(self, precision_policy, backend): | |||
| # Construct the velocity vectors and weights | |||
| c = np.array([ci for ci in itertools.product([-1, 0, 1], repeat=3) if np.sum(np.abs(ci)) <= 2]).T | |||
There was a problem hiding this comment.
If you want to make this consistant, pls make sure that D2Q9 is similar. But overall, again this could change in the future, as you have store them symmetrically and avoid storing values for opp index.
There was a problem hiding this comment.
It is consistent for all 3 lattices.
| @@ -53,9 +53,6 @@ def __init__( | |||
| mesh_vertices, | |||
| ) | |||
|
|
|||
| # Set the flag for auxilary data recovery | |||
There was a problem hiding this comment.
I am not sure. We created reconstruction originally for this BC!:
# Update the auxilary data for this BC using the neighbour's populations stored in f_aux and
# f_pre (post-streaming values of the current voxel). We use directions that leave the domain
# for storing this prepared data.
If you don[t recover, you'll 100% overwrite this data...
|
This PR fixes this issue: |
Contributing Guidelines
Description
I noticed that mlups_3d which relies on D3Q19 now gives tiny bit faster results (~ 0.1%) which could be due to item 2 above.
Type of change
How Has This Been Tested?
Linting and Code Formatting
Make sure the code follows the project's linting and formatting standards. This project uses Ruff for linting.
To run Ruff, execute the following command from the root of the repository:
ruff check .