Skip to content
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

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions unit/ctest_rect_grid.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,139 @@ void test_grid_2d()
}
}

void test_find_cell_1x(){
Copy link
Collaborator

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.

double lower[] = {0.0}, upper[] = {5.0};
int cells[] = {5};
double point[] = {2.5};
bool pickLower = false;
int idx = 1;
const int *ptr = &idx;
const int *knownIdx[1] = {NULL};
const int *knownIdx2[1] = {ptr};
int cellIdx[]={0};
int correctIdx[1]={3};
struct gkyl_rect_grid grid;

gkyl_rect_grid_init(&grid, 1, lower, upper, cells);
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<1;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

point[0]=2.0+1e-15;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<1;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

pickLower=true;
correctIdx[0]=2;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<1;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

correctIdx[0] = idx;
point[0] = 0.2;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx2, cellIdx);
for(int i=0;i<1;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

}

void test_find_cell_2x(){
double lower[] = {0.0, -10.0}, upper[] = {5.0, 10.0};
int cells[] = {5, 20};
double point[] = {2.5, 1.3};
bool pickLower = false;
int idx = 18;
const int *ptr = &idx;
const int *knownIdx[2] = {NULL,NULL};
const int *knownIdx2[2] = {NULL,ptr};
int cellIdx[]={0,0};
int correctIdx[2]={3,12};
struct gkyl_rect_grid grid;

gkyl_rect_grid_init(&grid, 2, lower, upper, cells);
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<2;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

point[0]=2.0;
point[1]=1.0;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<2;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

pickLower=true;
correctIdx[0]=2;
correctIdx[1]=11;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<2;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

pickLower=false;
correctIdx[0]=3;
correctIdx[1]=idx;
point[1]=7.5;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx2, cellIdx);
for(int i=0;i<2;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

}

void test_find_cell_3x(){
double lower[] = {0.0, -10.0, 1.3}, upper[] = {5.0, 10.0, 2.5};
int cells[] = {5, 20, 100};
double point[] = {2.5, 1.3, 1.4};
bool pickLower = false;
int idx = 18, idx2 = 35;
const int *ptr = &idx, *ptr2 = &idx2;
const int *knownIdx[] = {NULL,NULL,NULL};
const int *knownIdx2[] = {NULL,ptr,ptr2};
int cellIdx[]={0,0,0};
int correctIdx[]={3,12,9};
struct gkyl_rect_grid grid;

gkyl_rect_grid_init(&grid, 3, lower, upper, cells);
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<3;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

point[0]=2.0;
point[1]=1.0+1e-15;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<3;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

pickLower=true;
correctIdx[0]=2;
correctIdx[1]=11;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx, cellIdx);
for(int i=0;i<3;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

pickLower=false;
correctIdx[0]=3;
correctIdx[1]=idx;
correctIdx[2]=idx2;
point[1]=7.5;
point[2]=1.71;
gkyl_rect_grid_find_cell(&grid, point, pickLower, knownIdx2, cellIdx);
for(int i=0;i<3;i++){
TEST_CHECK( cellIdx[i]==correctIdx[i]);
}

}

void test_grid_io()
{
double lower[] = {1.0, 1.0}, upper[] = {2.5, 5.0};
Expand Down Expand Up @@ -79,6 +212,9 @@ void test_cu_grid_2d()

TEST_LIST = {
{ "grid_2d", test_grid_2d },
{ "grid_find_cell_1x", test_find_cell_1x },
{ "grid_find_cell_2x", test_find_cell_2x },
{ "grid_find_cell_3x", test_find_cell_3x },
{ "grid_io", test_grid_io },
#ifdef GKYL_HAVE_CUDA
{ "cu_grid_2d", test_cu_grid_2d },
Expand Down
13 changes: 13 additions & 0 deletions zero/gkyl_rect_grid.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

/**
Copy link
Collaborator

Choose a reason for hiding this comment

The 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,...).
Expand Down
7 changes: 7 additions & 0 deletions zero/gkyl_rect_grid_priv.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
#pragma once
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Collaborator

Choose a reason for hiding this comment

The 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[]);
134 changes: 134 additions & 0 deletions zero/rect_grid.c
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,
Expand All @@ -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){
Copy link
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not use Fortran style :) I agree with Mana

Copy link
Collaborator

Choose a reason for hiding this comment

The 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.
If you agree, please change throughout.

int nDim = grid->ndim;
int searchNum = 0;
int searchDim[GKYL_MAX_DIM], *idxOut;
Copy link
Collaborator

Choose a reason for hiding this comment

The 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++){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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++){
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here and elsewhere:

  • please remove the curly brackets in for loops that contain a single instruction.
  • Leave at least 2 spaces between code and comment.
  • Leave 1 space after // in your comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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
for (int j=1; j<b; j++) {
x=i+j;
.
.
.
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The 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);
Copy link
Collaborator

Choose a reason for hiding this comment

The 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).
There's a function like GKYL_MAX or something that you can use instead (see zero/gkyl_util.h).

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){
Copy link
Collaborator

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 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[]){
Copy link
Collaborator

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 describing this function and its inputs/outputs.

int nDim, checkIdx;
const double *dx, *lower;
nDim = grid -> ndim;
double testarr[nDim];
setbuf(stdout, NULL);
Copy link
Collaborator

Choose a reason for hiding this comment

The 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)
{
Expand Down
Loading