-
Notifications
You must be signed in to change notification settings - Fork 3
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
Sensei2 ghost libsim #3
base: master
Are you sure you want to change the base?
Conversation
…ome changes so we don't have empty vtkMultiBlockDataSet blocks.
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.
number of blocks tells the total across all ranks, then non-local blocks are nullptrs. that's the convention in vtk.
- this->Mesh->SetNumberOfBlocks(n_ranks);
- this->Mesh->SetBlock(rank, block);
+ this->Mesh->SetNumberOfBlocks(1);
+ this->Mesh->SetBlock(0, block);
this change is no good, might work with liibsiom but breaks the rules, probably cause paraview to crash or produce incorrect output.
y->Delete(); | ||
z->Delete(); | ||
this->Mesh = vtkSmartPointer<vtkMultiBlockDataSet>::New(); | ||
this->Mesh->SetNumberOfBlocks(1); |
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.
this should be n_ranks, because the way pjacobi decomposes the domain each rank has 1 block
z->Delete(); | ||
this->Mesh = vtkSmartPointer<vtkMultiBlockDataSet>::New(); | ||
this->Mesh->SetNumberOfBlocks(1); | ||
this->Mesh->SetBlock(0, block); |
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.
should be SetBlock(rank, block)
y->Delete(); | ||
z->Delete(); | ||
this->Mesh = vtkSmartPointer<vtkMultiBlockDataSet>::New(); | ||
this->Mesh->SetNumberOfBlocks(1); |
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.
should be
this->Mesh->SetNumberOfBlocks(n_ranks)
this->Mesh->SetBlock(rank, block)
gn->SetNumberOfTuples(nx*ny); | ||
gn->SetName(GHOST_NODE_ARRAY_NAME().c_str()); | ||
unsigned char *gptr = (unsigned char *)gn->GetVoidPointer(0); | ||
memset(gn->GetVoidPointer(0), 0, nx*nx*sizeof(unsigned char)); |
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.
should be nx*ny
for( ; i < nx; ++i) | ||
gptr[j*nx+i] = ghost; | ||
} | ||
block->GetPointData()->SetScalars(gn); |
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.
use AddArray here, SetScalars should be used for the array you want to visualize
memset(gn->GetVoidPointer(0), 0, nx*nx*sizeof(unsigned char)); | ||
unsigned char ghost = 1; | ||
// Left column | ||
if(this->sim->rankx > 0) |
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.
all the patches have full set of ghost nodes, so I think these tests on rank is not needed
also want to mention that using rectilnear mesh add substantial performance hit in ParaView for some operations such as stream line tracing. I have profiled this before and image data is much much faster than the same mesh represented in rectilinear form. It has to do with cell locators and having to do explicit searches through coordinate axes in rectilinear meshes. I realize that rectilinear is currently the only option in visit, but we want to hold this up as an example of best practices we might want to leave it as image data? ON the other hand we currently do not have a test case for rectilinear meshes, so changing it would be beneficial in that regard. I don't have strong feeling either way. @jfavre may have some opinion about this as he is using this in tutorials at cscs. as a related question, isn't about time VisIt supports image data? I have thought "yes" for a long time, and while it would be tedious to make that change, I think it is likely straight forward one? |
…es the same mesh as the rectilinear version. Changed vtkMultiBlockDataSet code so we set the rank dataset and have many empties. Fixed nx*nx typo.
I fixed the vtkMultiBlockDataset issues. I was not aware of having to make the dataset mostly empty except for the local data. I guess when Catalyst runs on 1M ranks, it makes 1M-1 empty pointers then. Bleh. I added vtkImageData code back in and adjusted the extents, etc so it makes the same picture as the vtkRectilinearMesh which was using the sim's actual coordinate arrays. I had changed it to vtkRectilinearGrid because I did not believe the vtkImageData's coordinates when I looked at things in VisIt. I made the recommended changes to the ghost nodes array. You're right. I did a temporary test where the temperature array returns the ghost nodes and they now appear around the edges of all domains. |
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.
only thing I see now is extents/coordinates not putting origin of the valid mesh at 0,0
#ifdef USE_IMAGE_DATA | ||
int Extent[6]; | ||
double Origin[3], Spacing[3]; | ||
Extent[0] = this->sim->rankx*this->sim->bx; |
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.
I'd like to keep the origin of the valid cells in the mesh at (0,0), which with grid spacing of 1 puts the origin of the mesh with ghost zones at (-1,-1). I think the rectilinear mesh should do the same.
Spacing[1] = this->sim->cy[1] - this->sim->cy[0]; | ||
Spacing[2] = 0.; | ||
|
||
// Origin on rank 0 using cx[0], cy[0] is zero. |
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.
the origin of valid part of the mesh should be located at 0,0, with ghost zones origin will be negative (-dx*ng, -dy*ng)
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.
I was just exposing the coordinates as computed in the simulation's AllocateGridMemory routine. The minimum values are zero, not negative. If that's not what it ought to be then the simulation should be different - not the adaptor.
Rank 0: cx={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561, 0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293, 0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024, 0.463415, 0.487805, 0.512195}
Rank 0: cy={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561, 0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293, 0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024, 0.463415, 0.487805, 0.512195}
Rank 1: cx={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561, 0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293, 0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024, 0.463415, 0.487805, 0.512195}
Rank 1: cy={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366, 0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098, 0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829, 0.951219, 0.975610, 1.000000}
Rank 2: cx={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366, 0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098, 0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829, 0.951219, 0.975610, 1.000000}
Rank 2: cy={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561, 0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293, 0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024, 0.463415, 0.487805, 0.512195}
Rank 3: cx={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366, 0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098, 0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829, 0.951219, 0.975610, 1.000000}
Rank 3: cy={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366, 0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098, 0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829, 0.951219, 0.975610, 1.000000}
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.
@BradWhitlock see comment below
Spacing[1] = this->sim->cy[1] - this->sim->cy[0]; | ||
Spacing[2] = 0.; | ||
|
||
// Origin on rank 0 using cx[0], cy[0] is zero. |
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.
the origin of valid part of the mesh should be located at 0,0, with ghost zones origin will be negative (-dx*ng, -dy*ng)
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.
Sorry guys, I have been in backlog mode ever since I finished the tutorial with Burlen, teaching a 3rd day event, then attending a class myself. I cannot find the time to properly interact with you until after April 9 (will be on vacation for the Easter week). Cheers. Jean
Hi Brad,
I don't disagree with your assessment of where the bug is. My issue with
your change is that you introduced a new bug into the uniform mesh case.
Take a look at how the extents were set before your change. It took me
some effort to get the extents correct, and hence I do not want to see
that effort undone. Could you fix the simulation so that the coordinate
axes are generated to reflect the ghost nodes correctly?
Burlen
…On 03/30/2018 10:50 AM, Brad Whitlock wrote:
***@***.**** commented on this pull request.
------------------------------------------------------------------------
In Jacobi/Sensei/C/solution/JacobiDataAdaptor.cxx
<#3 (comment)>:
> - block->SetSpacing(this->Spacing);
+#ifdef USE_IMAGE_DATA
+ int Extent[6];
+ double Origin[3], Spacing[3];
+ Extent[0] = this->sim->rankx*this->sim->bx;
+ Extent[1] = Extent[0] + this->sim->bx + 2 - 1;
+ Extent[2] = this->sim->ranky*this->sim->by;
+ Extent[3] = Extent[2] + this->sim->by + 2 - 1;
+ Extent[4] = 0;
+ Extent[5] = 0;
+
+ Spacing[0] = this->sim->cx[1] - this->sim->cx[0];
+ Spacing[1] = this->sim->cy[1] - this->sim->cy[0];
+ Spacing[2] = 0.;
+
+ // Origin on rank 0 using cx[0], cy[0] is zero.
I was just exposing the coordinates as computed in the simulation's
AllocateGridMemory routine. The minimum values are zero, not negative.
If that's not what it ought to be then the simulation should be
different - not the adaptor.
Rank 0: cx={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561,
0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293,
0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024,
0.463415, 0.487805, 0.512195}
Rank 0: cy={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561,
0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293,
0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024,
0.463415, 0.487805, 0.512195}
Rank 1: cx={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561,
0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293,
0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024,
0.463415, 0.487805, 0.512195}
Rank 1: cy={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366,
0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098,
0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829,
0.951219, 0.975610, 1.000000}
Rank 2: cx={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366,
0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098,
0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829,
0.951219, 0.975610, 1.000000}
Rank 2: cy={, 0.000000, 0.024390, 0.048780, 0.073171, 0.097561,
0.121951, 0.146341, 0.170732, 0.195122, 0.219512, 0.243902, 0.268293,
0.292683, 0.317073, 0.341463, 0.365854, 0.390244, 0.414634, 0.439024,
0.463415, 0.487805, 0.512195}
Rank 3: cx={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366,
0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098,
0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829,
0.951219, 0.975610, 1.000000}
Rank 3: cy={, 0.487805, 0.512195, 0.536585, 0.560976, 0.585366,
0.609756, 0.634146, 0.658537, 0.682927, 0.707317, 0.731707, 0.756098,
0.780488, 0.804878, 0.829268, 0.853658, 0.878049, 0.902439, 0.926829,
0.951219, 0.975610, 1.000000}
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3 (comment)>, or
mute the thread
<https://github.com/notifications/unsubscribe-auth/AAHBbSDe6scHudZfI3mtJuCzBxTuyY29ks5tjnBNgaJpZM4S7rX2>.
|
Some changes to pjacobi to expose ghost nodes via SENSEI 2 API.