Sample implementation for ArbLattice#535
Conversation
llaniewski
left a comment
llaniewski
left a comment
There was a problem hiding this comment.
@lisataldir maybe a meeting would be in order?
| std::unique_ptr<Index[]> og_index; | ||
| std::unique_ptr<Index[]> nbrs; | ||
| std::unique_ptr<ZoneIndex[]> zones_per_node; | ||
| std::vector<Index> cart_index; |
There was a problem hiding this comment.
This seems to be only encode the position in a different way. Couldn't we just search in the coords?
| /// Calculation of the offset from x, y and z | ||
| int ArbLattice::Offset(int x, int y, int z) | ||
| { | ||
| return x+connect.nx*y + connect.nx*connect.ny*z; |
There was a problem hiding this comment.
This doesn't seem to be a good idea, as the size of the thing can be quite big. Even if we stick with this solution, this should be 64bit (see sizeL in region)
| #ifdef ADJOINT | ||
| setAdjSnapIn(aSnap); | ||
| #endif | ||
| lbRegion small = getLocalBoundingBox().intersect(over); |
There was a problem hiding this comment.
This seem to be wrong. The indexes in read from the file are in the global region, a this seems to take smaller (local) region.
| setAdjSnapIn(aSnap); | ||
| #endif | ||
| lbRegion small = getLocalBoundingBox().intersect(over); | ||
| auto it = std::find(connect.cart_index.begin(), connect.cart_index.end(), Offset(small.dx, small.dy, small.dz)); |
There was a problem hiding this comment.
This is the only place these indexes are really used. Couldn't it be replaced with search for the closest point in coords?
| lbRegion small = getLocalBoundingBox().intersect(over); | ||
| auto it = std::find(connect.cart_index.begin(), connect.cart_index.end(), Offset(small.dx, small.dy, small.dz)); | ||
| int lid = std::distance(connect.cart_index.begin(), it); | ||
| launcher.SampleQuantity(quant, lid, buf, scale, data); |
There was a problem hiding this comment.
It doesn't seem very efficient to search this point every time.
| int x = CudaBlock.x + small.dx; | ||
| int y = CudaBlock.y + small.dy; | ||
| int z = CudaBlock.z + small.dz; | ||
| int x = (CudaBlock.x + small.dx) %container.nx; |
There was a problem hiding this comment.
Why did this appear here? x,y,z should always be in container's region, and modulu ("%") is very expensive and generally not very reliable.
| return EXIT_SUCCESS; | ||
| }; | ||
|
|
||
| const auto init_arbitrary = [&](const Lattice<ArbLattice>* lattice) { |
There was a problem hiding this comment.
There seems to be lot of overlap between these two functions. Maybe something could be generalized here?
2 participants
I added a handler Sample for arbitrary lattice (took time to pull it because the implementation is not really efficient)