Find cell #212
Find cell #212
Conversation
| @@ -29,6 +29,19 @@ struct gkyl_rect_grid { | |||
| void gkyl_rect_grid_init(struct gkyl_rect_grid *grid, int ndim, | |||
| const double *lower, const double *upper, const int *cells); | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
@jRoeltgen fix typos in these comments
| @@ -0,0 +1,7 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
@jRoeltgen
i don't think you need a private header just to declare the signature of private functions. Either just place those functions in the .c file before they get used, or put the functions themselves in the private header.
There was a problem hiding this comment.
although I understand why one may want to do it this way, we just haven't AFAIK, so let's stick to the current model for now.
| void gkyl_rect_grid_find_cell(struct gkyl_rect_grid *grid, const double *point, bool pickLower, const int **knownIdx, int *cellIdx){ | ||
| int nDim = grid->ndim; | ||
| int searchNum = 0; | ||
| int searchDim[GKYL_MAX_DIM], *idxOut; |
There was a problem hiding this comment.
I think you are not using idxOut. Delete.
| @@ -22,6 +24,138 @@ gkyl_rect_grid_init(struct gkyl_rect_grid *grid, int ndim, | |||
| } | |||
| } | |||
|
|
|||
| GKYL_CU_DH | |||
| void gkyl_rect_grid_find_cell(struct gkyl_rect_grid *grid, const double *point, bool pickLower, const int **knownIdx, int *cellIdx){ | |||
There was a problem hiding this comment.
If I understand correctly, you are declaring a bunch of variables at the beginning of your function. That's a common style in Fortran, and some C programmers. However I (or maybe even we) prefer that you declare variables as needed. So please move them to right before they are used. The benefit of this latter style is that you keep the definition closer to where it is used, and don't require the reader to memorize the definition to later understand or check its (correct) use
There was a problem hiding this comment.
Do not use Fortran style :) I agree with Mana
| double lowerInDir[nDim], upperInDir[nDim]; | ||
| double low, high; | ||
|
|
||
| for(int d=0; d<nDim; d++){ |
There was a problem hiding this comment.
here and elsewhere, please use spacing like other gkeyll code.
In this case leave a space after for. Below leave a space after if, and before and after else.
| const double *dx, *lower; | ||
| nDim = grid -> ndim; | ||
| double testarr[nDim]; | ||
| setbuf(stdout, NULL); |
| } | ||
|
|
||
| GKYL_CU_DH | ||
| bool isInCell(const struct gkyl_rect_grid *grid, const double *pIn, int *iIn, int *dimTrans[1], const int **knownIdx){ |
There was a problem hiding this comment.
please add some (very brief) comments describing this function and its inputs/outputs.
| } | ||
|
|
||
| GKYL_CU_DH | ||
| void InDir(const struct gkyl_rect_grid *grid, int *iIn, int *dimTrans[1], const int **knownIdx, double lowerInDir[], double upperInDir[]){ |
There was a problem hiding this comment.
please add some (very brief) comments describing this function and its inputs/outputs.
| @@ -22,6 +24,138 @@ gkyl_rect_grid_init(struct gkyl_rect_grid *grid, int ndim, | |||
| } | |||
| } | |||
|
|
|||
| GKYL_CU_DH | |||
| void gkyl_rect_grid_find_cell(struct gkyl_rect_grid *grid, const double *point, bool pickLower, const int **knownIdx, int *cellIdx){ | |||
There was a problem hiding this comment.
Biggest change I would recommend is to change knownIdx from a ** to a *. The latter will be easier to work with. We can assume that entries in knownIdx <0 are not known (like nil in the Lua code), and entries >=0 are known indices.
If you agree, please change throughout.
| @@ -35,6 +35,139 @@ void test_grid_2d() | |||
| } | |||
| } | |||
|
|
|||
| void test_find_cell_1x(){ | |||
There was a problem hiding this comment.
please add some (very brief) comments to your testing functions to at least say which case you are testing in each bit of code. So future readers know the purpose of each bit.
|
@jRoeltgen you technically haven't tested your method on GPUs. Although you may have run on/with a GPU, it executed the code on the CPU. To test it on the GPU you'll have to add code to uni/ctest_rect_grid_cu_.cu |
|
Please do not use camelCase for names. Please be consistent with the rest of the code and use underscores to name functions. Please make these changes before we consider the PR. Prefix all function names with gkyl_ in all public facing APIs |
Only CPU version has been written/tested. Binary search to find cell indices in rect_grid.