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

DBOPT_COORDSYS problems in test data and VisIt #17187

Open
markcmiller86 opened this issue Dec 9, 2021 · 1 comment
Open

DBOPT_COORDSYS problems in test data and VisIt #17187

markcmiller86 opened this issue Dec 9, 2021 · 1 comment
Labels
bug Something isn't working impact medium Productivity partially degraded (not easily mitigated bug) or improved (enhancement) likelihood medium Neither low nor high likelihood priority a priority ticket reviewed Issue has been reviewed and labeled by a developer

Comments

@markcmiller86
Copy link
Member

markcmiller86 commented Dec 9, 2021

Problem with data

The test file curv2d.silo indicates its mesh is using a coordinate system of cylindrical. However, if you look at the actual code,

for (j = 0; j < ny + 1; j++)
{
x[j * sn2 + i * sn1] = r * cos(theta);
y[j * sn2 + i * sn1] = r * sin(theta);
r += dr;
}

that computes the coordinate data, it is not putting r and theta into the coordinate arrays. It is computing the cartesian coordinates and putting those values in the coordinate arrays.

Next, even when I have corrected for this, VisIt will wind up plotting the data incorrectly. It will plot the mesh as though the coordinate data is x/y not r/theta. I have a new example I've started as part of quad_disk.C where I do something similar.

Problem with VisIt

I can see a few answers here...

  1. The plugin should be smart enough to convert from cylindrical to cartesian before VisIt even sees the data.
  2. The plugin should inform VisIt that the mesh coordinates are in cylindrical and VisIt should be smart enough to handle that.

I would appear we do both of these. For 2D Unstructured and Structured mesh with coordsys set to DB_CYLINDRICAL, we set the mesh_type to AVT_RZ in avtMeshMetaData. When we eventually go to read the coordinates of the mesh, I think (I have yet to confirm this) we pass the coordinate data from the file onto VisIt unchanged meaning that VisIt is getting arrays of r/theta values.

if (ndims ==2 && um->coord_sys == DB_CYLINDRICAL)
mct = AVT_RZ;
else
mct = AVT_XY;

For Curves and 3D quad meshes, we appear to convert to cartesian in the plugin,

if (qm->ndims == 3 && qm->coord_sys == DB_CYLINDRICAL)
ConvertQuadCylindricalCoordinates((double *) pts, nx, ny, nz,
(double *) qm->coords[0], (double *) qm->coords[1], (double *) qm->coords[2]);
else
CopyQuadCoordinates((double *) pts, nx, ny, nz, qm->major_order,
(double *) qm->coords[0], (double *) qm->coords[1],
qm->ndims <= 2 ? 0 : (double *) qm->coords[2]);
}

if (cur->coord_sys == DB_SPHERICAL || cur->coord_sys == DB_CYLINDRICAL)
{
for (int i = 0 ; i < cur->npts; i++)
{
T x = px[i] * cos(py[i]);
T y = px[i] * sin(py[i]);
xc->SetValue(i, x);
yv->SetValue(i, y);
}

I don't think we handle 3D unstructured meshes but then why would we. There isn't the possibility of saving any in coordinate array storage for 3D data when its an unstructured mesh.

This is freakishly inconsistent, leads to added complexity and hard to justify. IMHO, downstream of the plugin, pretty much everything is kinda sorta expecting cartesian. So, it makes sense to me that we convert to cartesian in the plugin. That said, what does user want returned for coordinate data when s/he picks? I assume user wants their native coordinate data which converting subverts. Also relevant but less important collinear coordinates in cylindrical will not be collinear in cartesian and vice-versa so there is an increase in memory requirement when we convert. Think of it as adding 2-3 additional problem sized variables for the x, y and if 3D z coordinate data.

@markcmiller86 markcmiller86 added bug Something isn't working likelihood medium Neither low nor high likelihood impact medium Productivity partially degraded (not easily mitigated bug) or improved (enhancement) labels Dec 9, 2021
@brugger1
Copy link
Collaborator

Fixing the issue with curv2d.silo is definitely a good idea. Doing these transforms in the transform manager may impact existing data sets, so we should think about this further and only do it on a minor release.

@brugger1 brugger1 added reviewed Issue has been reviewed and labeled by a developer priority a priority ticket labels Dec 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working impact medium Productivity partially degraded (not easily mitigated bug) or improved (enhancement) likelihood medium Neither low nor high likelihood priority a priority ticket reviewed Issue has been reviewed and labeled by a developer
Projects
None yet
Development

No branches or pull requests

2 participants