Fix mesh line annotation for triangulations with a side in the plane #1437
Conversation
|
You should be able to back out #1396 too :) |
|
@ngoldbaum Did you ever ID the cause of the seg faults? In studying this bug, I never got any seg faults... |
|
Ah, I was under the impression that #1273 was filed after andrew tried to track down the cause of he seg faults. Have you tried just running the test 100 times in a row? I can try reproducing the seg faults tomorrow. I know we ran into them on fido so we can try there if all else fails. |
|
Yea, I don't know the history behind #1273...No, I haven't tried running it many times in a row, although I did probably end up running it 100 times in total :-) If you want to give it a shot tomorrow, please have at it! |
|
Thanks for tracking this down! I think this is a separate issue from #1273. |
|
So the #1273 script with
This PR fixes element boundaries in the plane issue but there's still some interesting mesh line annotations that I'm trying to wrap my head around as shown in this sweep of slices (probably have to open the picture outside the browser to really see it):
@atmyers I'm trying to convince myself that this is just some interesting artifact of the fake hexahedral mesh. Have you ever gone backwards and visualized a mesh (say as a wireframe) given only coordinates and indices? E.g. my first thought would be to write out to exodus and open with paraview which has a nice wireframe visualization feature. I believe that it's possible looking at the Exodus API, but maybe a lot of work unless someone's already done it. Maybe there's a |
|
That windows test failure is real. Let me know if you need a hand fixing it. I think just changing the function in the error to accept |
|
Might also make sense to use the http://cython.readthedocs.io/en/latest/src/userguide/fusedtypes.html#built-in-fused-types I have a windows machine on my desk too if you want to be able to debug without waiting for appveyor. You can also remote desktop into appveyor nodes, I used that yesterday: https://www.appveyor.com/docs/how-to/rdp-to-build-worker/ |
|
Unfortunately, I'm having a hard time finding where exactly the fake hexahedral mesh came from. I thought I had exported it from a MOOSE sample dataset somehow, but I can't seem to find the one it came from anywhere. It's possible that this dataset is ill-formed somehow (for example, if the node numberings don't use the normal ExodusII conventions). Is there any behaviour like this for any of the MOOSE datasets? If not, I'd be inclined to regenerate the fake dataset. |
|
If memory serves, it was indeed generated from scratch, and might be time to replace it with a new one. |
|
Alex mentioned to me at our group meeting that the new sample dataset doesn't have the issues he was seeing in his comment two days ago. Once the tests pass I think this is ready to go in. |
yt/utilities/lib/geometry_utils.pyx
Outdated
| if count == 2: | ||
| nlines += 1 | ||
| elif count == 3: | ||
| nlines += 2 | ||
| raise RuntimeError("I'm impressed you got a plane to intersect all three " | ||
| "legs of a triangle") |
There was a problem hiding this comment.
Can you reword this? Imagine you're a random user who hits this error and doesn't understand the guts of what's going on here.
There was a problem hiding this comment.
Haha yes. I get snarky when a bug has foiled me for a few days...
yt/utilities/lib/geometry_utils.pyx
Outdated
| if p3[0] * p3[1] < 0: count += 1 | ||
| if p3[1] * p3[2] < 0: count += 1 | ||
| if p3[2] * p3[0] < 0: count += 1 | ||
| p2[j] = copysign(1.0, triangles[i, j, ax] - coord + 0) |
There was a problem hiding this comment.
Maybe add a comment explaining why you're adding zero (if it's necessary).
| https://github.com/yt-project/yt/pull/1437 this test falsely | ||
| yielded three annotation lines, whereas the correct result is four | ||
| corresponding to the four edges of the top hex face. | ||
| ''' |
There was a problem hiding this comment.
Can you convert this docstring into a comment? Nose interprets the first line of a docstring on a test as the "name" of the test. That means that right now this test shows up in the test runner like this:
This test tests mesh line annotation where a z=0 slice ... ok (0.1249s)
Which I think is less useful (and greppable) than just printing the module and test function name.
Fix mesh line annotation for triangulations with a side in the plane
PR Summary
Fixes issues #1273 where mesh line annotations are thrown off when a side of the triangulation lies in the slice plane. AFAICT the issue is fixed by making sure that
copysigndoesn't receive a0as the second argument for one node and a-0as the second argument for another node.PR Checklist
Need to add a fixed bug test