Used center of f_1 as an additional storage and also fixed some bugs

[hsalehipour]

Contributing Guidelines

• I have read and understood the CONTRIBUTING.md guidelines

Description

Added feature:

1. Added central location of output population (f_1) as another storage space for storing BC auxilary data since that's also available.
2. Used this newly added space for allocating single auxilary data of ZouHe/Regularized which simplified the code and removed for-loops over lattice direction for reading the prescribed values.

Fixed the following bugs:

1. Flow over sphere was not working with prescribed pressure in JAX
2. Flow over sphere was not working with FP64FP64 in WARP
3. Regularized or ZouHe were not working with prescribed velocity of ZERO vector associated with no-slip BC (straight wall).

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Documentation update

How Has This Been Tested?

• All pytest tests pass

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 .

• Ruff passes

[mehdiataei]

This seems unnecessary, and I’m not even sure it won’t cause issues in some scenarios that you may need post/pre collision values as the center value is updated during collision. Could you please explain what problem this extra data is intended to solve?

At the very least, it introduces a lot of confusion and deviates from the usual definition of using "unknown population" for storage. Regarding the central population, @massimim dicussed the possibility of using a different buffer for central populations in the future, so this approach will not be compatible in that context.

[mehdiataei]

Same comment

[hsalehipour]

Are you suggesting that this BC (zouhe) might need more than 1 aux variable? I am not sure what you mean by "more general".

[hsalehipour]

This seems unnecessary, and I’m not even sure it won’t cause issues in some scenarios that you may need post/pre collision values as the center value is updated during collision. Could you please explain what problem this extra data is intended to solve?

At the very least, it introduces a lot of confusion and deviates from the usual definition of using "unknown population" for storage. Regarding the central population, @massimim dicussed the possibility of using a different buffer for central populations in the future, so this approach will not be compatible in that context.

We always have these additional storage spaces at our disposal say in the pull method: (1) unknown directions of the output buffer (2) known directions of the input buffer and (3) center of the output. The known directions of the input (or f_0 in our notation) is often needed to store post-collision values for the BC and hence we don't use that. Recently we have used unknown directions of output (or f_1) in Extrapolation outflow BC and also for storing single values of normal vel and density in Regularized and ZouHe. However, we had never used the center location of the output. This is all part of our novel complex BC approach (you can confirm with @massimim who actually explained this all to me). Now this PR introduces the use of central location which you can see has generic retrival for 1 and +1 aux data. Only in Regularized and ZouHe which needs one aux data, we use center only. In other BCs (not added yet) we need not only the unknown directions but also the center location for storing aux data. Yes you can store index of missing directions to avoid the for loop and it would work but that's unnecessary here since we do have the center location that we should leverage and it is always at index 0.

I hope this answers your questions about "what problem this extra data is intended to solve".

[mehdiataei]

I don't think this is correct. This will cause information loss and the core idea of using aux data is that you use "UNUSED" populations to store these values.

Say that I come up with a new BC tomorrow that is applied during streaming, and requires pre-collision populations. How can I retrieve the value of pre-collision at location 0 at the beginning of the iteration?

[hsalehipour]

The center location of OUTPUT (or f_1) is UNUSED and no one needs it just like the missing directions of f_1. This has nothing to do with requiring the pre-collision (or post-streaming) populations because they are already available in the thread (those desgingated with _f_post_stream in the code). No one uses centeral position of f_1 just like how the missing direcctions of f_1 are unused. Please think about this more carefully.

[mehdiataei]

This doesn't answer my question. "Say that I come up with a new BC tomorrow that is applied during streaming, and requires pre-collision populations. How can I retrieve the value of pre-collision at location 0 at the beginning of the iteration?" At the beginning of the iteration the registers are empty, so they're not stored there.

[hsalehipour]

haha! it does not answer your question because you changed / editted your question :) But again we store aux data in f_0 at the end of the loop through apply_aux_recovery_bc function. Then f_0 is swapped with f_1 and hence at the beginning of the next iteration all aux data are in f_1 and are read into thread and designated by _f1_thread. So now if there is a hypothetical BC that is applied during streaming and needs pre-streaming or post-collision data, that information is jut available in _f0_thread and has nothing to do with _f1_thread. If the hypothetical BC needs post-streaming (or pre-collision) values they are again available in _f_post_stream.

In any case, please think about it this way: any argument you want to make against center location of f_1 by some hypothetical BC is equally applicable to the missing direction of f_1. I don't see any issues even in case of the two hypothetical scenarios you have mentioned so far as I have explained above.

[mehdiataei]

The difference is that the missing directions are reconstructed, the center is not. If that information is needed (for any reason whatsoever), it is lost as it is overwritten.

[hsalehipour]

NO! It is not lost! Of course we ALWAYS need the data at the center of the lattice but that is not overwritten by this method. We have two fields f_0 and f_1. Generally speaking in the two pop approach (as you know) f_0 is really all we need during an LBM step and f_1 is really needed only for storage. These tricks we are introducing for storing aux data of complex BCs is really about how to use f_1 for storage more wisely.

[mehdiataei]

If you will never need values of f_1 for any physical computations, you should be able to use the full extra field as additional storage, as long as you swap the whole cell in apply_aux_recovery_bc before storing f_1.

Hypothetically if you need them (like the example I gave, which is the pre-collision values at time t, given that the streaming operation does nothing at index=0), then overwriting them in apply_aux_recovery_bc will cause issues.

[mehdiataei]

But we don't do that, because it will cause race condition in the streaming. Again, if pre-collision values at time t is needed for any computations, the overwrite will lose that value. We don't currently use it in our BCs as far as I can tell.

[mehdiataei]

Isn't f_1 the f_post in functional_pressure? In that case, in _get_fsum function, we're using f_1[0] to compute fsum_middle?

[mehdiataei]

[mehdiataei]

fsum_middle includes f_1[0], which instead of the population it is adding the aux data. Am I wrong here?

[mehdiataei]

I think you use _f_post not f_post...the namings are very confusing. We should fix this later. The buffer values are not the same as the register values and the same name will cause confusion. We should use f_0 and f_1 for the buffers always.

[mehdiataei]

Again this is my final argument for this:

1. The value of f_1 at index=0, equals to pre-collision value at time t (current timestep when the kernel is running). If this value is NEVER needed for any physical computations, we can use that storage. This is NOT the same as the missing populations, as we always reconstruct them.
2. If the value is needed, or could be needed, it is better to use the storage of the remaining missing directions and store the index for trivial retrieval in case of +1 aux value.

[hsalehipour]

I agree with the naming confusion and pushed a change.

[hsalehipour]

1. The value of f_1 at index=0, equals to pre-collision value at time t (current timestep when the kernel is running). If this value is NEVER needed for any physical computations, we can use that storage. This is NOT the same as the missing populations, as we always reconstruct them.

No the value of f_1 at index=0 at current time step, t, is equal to the values of f_0 at the previous time-step (because of the buffer swapping) which are in turn equal to the pre-streaming (or post-collision) values at t-1 for the same voxel. The most complicated BC we have so far which is closest to your hypothetical scenario is ExrapolationOutflow that needs previous time-step information of the neighbouring cells. If you look at update_bc_auxilary_data in that BC you will see that we read from f_0 which includes index 0 so we never read from f_1 unless a quantity is stored there that is needed. Of course we cannot store information in the known directions of f_1 because of the race conditioning with other threads but this is not true for the central location.

2. If the value is needed, or could be needed, it is better to use the storage of the remaining missing directions and store the index for trivial retrieval in case of +1 aux value.

I don't think we ever need that space.

[mehdiataei]

Revert this back to compute type instead.

[hsalehipour]

revering it back will cause lots of issues if you run this example with f64/f64.

[mehdiataei]

Hmm pls create an issue if it is a bug.

[mehdiataei]

same.

[hsalehipour]

revering it back will cause lots of issues if you run this example with f64/f64.

[mehdiataei]

hmm pls create an issue if it is a bug

[hsalehipour]

Applied the requested changes and pushed again.