-
Notifications
You must be signed in to change notification settings - Fork 4
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
Find cell #212
Find cell #212
Changes from all commits
5292120
38fc405
4201e21
837c297
b60b840
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jRoeltgen fix typos in these comments |
||
* Find cell indecies of point | ||
* | ||
* @param grid Grid object | ||
* @param point The point to find the cell incecies at | ||
* @param pickLower | ||
* @param knownIdx Any known indecies of where the point is | ||
* @param cellIdx Pointer to cell indecies | ||
* Asserts: point lies within cell(s) specified by knownIdx (if specified). | ||
*/ | ||
void gkyl_rect_grid_find_cell(struct gkyl_rect_grid *grid, const double *point, | ||
bool pickLower,const int **knownIdx, int *cellIdx); | ||
|
||
/** | ||
* Get cell-center coordinates. Note that idx is a 1-based cell index, | ||
* i.e. the lower-left corner is (1,1,...). | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
#pragma once | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jRoeltgen There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
#include <gkyl_rect_grid.h> | ||
|
||
bool isInCell(const struct gkyl_rect_grid *grid, const double *pIn, int *iIn, int *dimTrans[1], const int **knownIdx); | ||
|
||
void InDir(const struct gkyl_rect_grid *grid, int *iIn, int *dimTrans[1], const int **knownIdx, double lowerInDir[], double upperInDir[]); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
#include <assert.h> | ||
#include <stdint.h> | ||
#include <math.h> | ||
|
||
#include <gkyl_alloc.h> | ||
#include <gkyl_rect_grid.h> | ||
#include <gkyl_rect_grid_priv.h> | ||
|
||
void | ||
gkyl_rect_grid_init(struct gkyl_rect_grid *grid, int ndim, | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not use Fortran style :) I agree with Mana There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
int nDim = grid->ndim; | ||
int searchNum = 0; | ||
int searchDim[GKYL_MAX_DIM], *idxOut; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you are not using idxOut. Delete. |
||
int *dimTrans[GKYL_MAX_DIM]; | ||
int plusminus[2] = {-1,1}; | ||
bool allLessEq = true; | ||
int iStart[GKYL_MAX_DIM], iEnd[GKYL_MAX_DIM], iMid[GKYL_MAX_DIM], iNew[GKYL_MAX_DIM]; | ||
int *cells, lowHighCellIdx[2*GKYL_MAX_DIM]; | ||
double lowerInDir[nDim], upperInDir[nDim]; | ||
double low, high; | ||
|
||
for(int d=0; d<nDim; d++){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. here and elsewhere, please use spacing like other gkeyll code. |
||
dimTrans[d] = (int*)malloc(1*sizeof(int)); | ||
if(knownIdx[d]==NULL){ | ||
searchDim[searchNum] = d; | ||
*dimTrans[d] = searchNum; | ||
searchNum = searchNum + 1; | ||
}else{ | ||
dimTrans[d] = NULL; | ||
cellIdx[d] = *knownIdx[d]; | ||
low = grid->lower[d]+(*knownIdx[d]-1)*grid->dx[d]; | ||
high = grid->lower[d]+(*knownIdx[d])*grid->dx[d]; | ||
assert(low<point[d]&&high>point[d]); | ||
} | ||
} | ||
|
||
cells = grid -> cells; | ||
for(int d=0; d<searchNum; d++){ | ||
iStart[d] = 1; | ||
iEnd[d] = cells[searchDim[d]]; | ||
iMid[d] = 0; | ||
iNew[d] = 0; | ||
} | ||
/* Below we use a binary search. That is, if the i-th coordinate of the point in | ||
* question is below(above) the ith-coordinate of the current mid-point, we search | ||
* the lower(upper) half along that direction in the next iteration. | ||
*/ | ||
while (allLessEq){ | ||
for(int d=0; d<searchNum; d++){ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and elsewhere:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should the curly brackets be neglected from the outer for loop in double for loops even when the inner for loop has multiple instructions (assuming the outer for loop only contains the inner for loop), such as the following? for (int i=1; i<a; i++) { // this curly bracket There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think not, good catch. An exception to my first bullet above is when that single instruction is another loop or switch |
||
iMid[d] = (iStart[d]+iEnd[d])/2;//Integer division intentional | ||
} | ||
|
||
if(isInCell(grid, point, iMid, dimTrans, knownIdx)){ | ||
//Check if neighboring cells also contain this point. | ||
for(int k=0; k<searchNum; k++){ | ||
iNew[k] = iMid[k]; | ||
lowHighCellIdx[searchDim[k]] = iMid[k]; | ||
lowHighCellIdx[nDim+searchDim[k]] = iMid[k]; | ||
} | ||
for(int i=0; i<searchNum; i++){ | ||
for(int j=0; j<2; j++){ | ||
iNew[i] = fmax(fmin(iMid[i] + plusminus[j],cells[i]),0); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think fmax and fmin are for floats/doubles (maybe I'm wrong). |
||
if(isInCell(grid, point, iNew, dimTrans, knownIdx)){ | ||
lowHighCellIdx[j*nDim+searchDim[i]] = iNew[i]; | ||
} | ||
} | ||
iNew[i] = iMid[i]; | ||
} | ||
if(pickLower){ | ||
for(int d=0; d<searchNum; d++){ | ||
cellIdx[searchDim[d]] = lowHighCellIdx[searchDim[d]]; | ||
} | ||
}else{ | ||
for(int d=0; d<searchNum; d++){ | ||
cellIdx[searchDim[d]] = lowHighCellIdx[nDim+searchDim[d]]; | ||
} | ||
} | ||
break; | ||
}else{ | ||
InDir(grid, iMid, dimTrans, knownIdx, lowerInDir, upperInDir); | ||
for(int d=0; d<searchNum; d++){ | ||
if(point[searchDim[d]] < lowerInDir[searchDim[d]]){ | ||
iEnd[d] = iMid[d]-1; | ||
}else if(point[searchDim[d]] > upperInDir[searchDim[d]]){ | ||
iStart[d] = iMid[d]+1; | ||
} | ||
} | ||
} | ||
|
||
for(int d=0; d<searchNum; d++){ | ||
if(iStart[d]>iEnd[d]){ | ||
allLessEq = false; | ||
break; | ||
} | ||
} | ||
} | ||
|
||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add some (very brief) comments describing this function and its inputs/outputs. |
||
double eps = 1.0e-14; | ||
int checkIdx; | ||
bool inCell = true; | ||
int nDim; | ||
nDim = grid -> ndim; | ||
double lowerInDir[nDim], upperInDir[nDim]; | ||
|
||
for(int d=0;d<nDim; d++){ | ||
lowerInDir[d]=0; | ||
upperInDir[d]=0; | ||
} | ||
InDir(grid, iIn, dimTrans, knownIdx, lowerInDir, upperInDir); | ||
for(int d=0; d<nDim; d++){ | ||
if(lowerInDir[d]-eps>pIn[d] || upperInDir[d]+eps<pIn[d]){ | ||
inCell = false; | ||
break; | ||
} | ||
} | ||
return inCell; | ||
} | ||
|
||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please add some (very brief) comments describing this function and its inputs/outputs. |
||
int nDim, checkIdx; | ||
const double *dx, *lower; | ||
nDim = grid -> ndim; | ||
double testarr[nDim]; | ||
setbuf(stdout, NULL); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is this? |
||
dx = grid -> dx; | ||
lower = grid -> lower; | ||
|
||
for(int d=0; d<nDim; d++){ | ||
checkIdx = knownIdx[d]==NULL ? iIn[*dimTrans[d]] : *knownIdx[d]; | ||
lowerInDir[d] = lower[d]+(checkIdx-1)*dx[d]; | ||
testarr[d] = lower[d]+(checkIdx-1)*dx[d]; | ||
upperInDir[d] = lower[d]+(checkIdx)*dx[d]; | ||
} | ||
} | ||
|
||
void | ||
gkyl_rect_grid_write(const struct gkyl_rect_grid *grid, FILE *fp) | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.