serial only intermediate version of SiloWriter::writeKullMesh capability#16
serial only intermediate version of SiloWriter::writeKullMesh capability#16dougmiller999 wants to merge 1 commit into
Conversation
|
Why do we need a special Kull mesh writer?
…On Thu, Nov 7, 2019, 11:58 AM dougmiller999 ***@***.***> wrote:
I don't *think* this breaks anything! I'm intending only to add the
function SiloWriter::writeKullMesh to the SiloWriter class. But, since it's
my first pull request, something will probably go wrong.
------------------------------
You can view, comment on, or merge this pull request online at:
#16
Commit Summary
- serial only intermediate version of SiloWriter::writeKullMesh
capability
File Changes
- *M* src/PYB11/polytopeMOD.py
<https://github.com/pbtoast/polytope/pull/16/files#diff-0> (41)
- *M* src/SiloWriter.hh
<https://github.com/pbtoast/polytope/pull/16/files#diff-1> (44)
- *M* src/SiloWriter_2d.cc
<https://github.com/pbtoast/polytope/pull/16/files#diff-2> (599)
Patch Links:
- https://github.com/pbtoast/polytope/pull/16.patch
- https://github.com/pbtoast/polytope/pull/16.diff
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#16>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCN6BVINU7QWXZUBDEZHPDQSRQOVANCNFSM4JKL47UA>
.
|
|
I don't know! I think when I started the motivation was that ShaPo wasn't quite ready to make meshes on its own, and Draco was definitely not going to make unstructured polygonal anything for us. POLYTOPE seemed like a good tool to start generating some polygonal meshes. I think I specialized it to Kull to avoid stepping on an existing mesh writer? I actually can't remember, it was so long ago. |
|
Worth trying to use Polytope's mesh writer and seeing what goes wrong? I seem to recall you saying something about unhappy faces, but that's something we should fix. |
|
So far as I know we only used the polytope writer for visit, so so long as
visit can render your modified format it would be fine too. Not a big deal,
just seems unnecessary to have two writers.
…On Thu, Nov 7, 2019, 12:09 PM dougmiller999 ***@***.***> wrote:
I don't know! I think when I started the motivation was that ShaPo wasn't
quite ready to make meshes on its own, and Draco was definitely not going
to make unstructured polygonal anything for us. POLYTOPE seemed like a good
tool to start generating some polygonal meshes.
I think I specialized it to Kull to avoid stepping on an existing mesh
writer? I actually can't remember, it was so long ago.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCN6BX7W3EK3XMOIY7PDLTQSRRYLANCNFSM4JKL47UA>
.
|
|
Ah, I think that was the deal. The existing mesh writer worked fine for Silo but it actually had some finicky thing that wasn't exactly correct in the Silo format that Kull insisted on. I added a new function because I wasn't sure if someone was using the existing writer and didn't want to mess it up for them. I think I intended, way back when, to have this conversation before committing anything to the real code base. Do we have a test suite I can run my writer against? If it passes both the polytope suite and Kull can read the output, then yup, two writers is dumb. |
|
Hrmmm, doesn't look like we use the Silo writer anywhere in the tests. Opportunities abound! Doug, do you have an "executive summary" of the modifications you made for Kull to read the mesh? Any reason we shouldn't incorporate them into the existing writer? Does it make assumptions specific to Kull and not to all arbitrary polygonal/hedral meshes? |
|
Yep, the reason there were no tests I believe is that the writer was only
used for visit. When we actually use polytope in Spheral and Kull it's as a
library -- both those codes then have their own native output and didn't
need polytopes version for anything.
…On Fri, Nov 8, 2019, 11:15 AM Jeffrey Johnson ***@***.***> wrote:
Hrmmm, doesn't look like we use the Silo writer anywhere in the tests.
Opportunities abound!
Doug, do you have an "executive summary" of the modifications you made for
Kull to read the mesh? Any reason we shouldn't incorporate them into the
existing writer? Does it make assumptions specific to Kull and not to all
arbitrary polygonal/hedral meshes?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCN6BWSTKKP7JSOTDIRWLDQSWUD5ANCNFSM4JKL47UA>
.
|
|
Ok, just reviewed the changes. Executive summary:
|
|
The only wierd things there to me are hardwiring the material to one, and
maybe the coordinate system. However, since polytope knows nothing of
materials or the way you view the coordinates provided, seems harmless-ish.
Mike.
…On Fri, Nov 8, 2019, 4:11 PM dougmiller999 ***@***.***> wrote:
Ok, just reviewed the changes. Executive summary:
1.
the output file is definitely different, but it is definitely a legit
SILO-compliant unstructured polygonal mesh, circa SILO March 2019. Also
Kull-compliant. And VisIt-compliant.
2.
Output file includes Material data, hard-wired to 1 material. Kull
must have needed that ("KMHNT").
3.
The DBOPT_COORDSYS option is included, hard-wired to DB_CARTESIAN.
KMHNT.
4.
The names of the files written out for the mesh are hard-wired
differently (KHMNT), and a directory is created where the files are dumped
(the directory can be named anything the user wants).
5.
The file writing changes were only made to the serial case; the
parallel code (the #ifdef HAVE_MPI clause) was not changed, so probably
(certainly) the parallel case isn't Kull-compliant.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCN6BSN6WJJUTDI5NJTZELQSXWY5ANCNFSM4JKL47UA>
.
|
|
Are we worried about the parallel file case? It doesn't seem like a high priority right now, but it'll have to be addressed eventually. |
|
Well we are going to need the parallel output to diagnose the parallel
tessellator at Duke point.
Mike
…On Fri, Nov 8, 2019, 4:21 PM dougmiller999 ***@***.***> wrote:
Are we worried about the parallel file case? It doesn't seem like a high
priority right now, but it'll have to be addressed eventually.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#16>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABCN6BSL5QPVRI74F33ACX3QSXYA5ANCNFSM4JKL47UA>
.
|
|
Agreed. I suppose the choices for you are,
All of these are ok with me, btw. I bow to the accumulated wisdom of the polytope elders. |
|
Well in a perfect world, number 3 would be best, but it has to be weighed against the burden we're placing on your enthusiasm, Tough Guy. In particular, it sounds like some things have to be decided which haven't definitely been yet (materials in silo files, coordinate systems, parallelism). I'm fine with 1 if you're willing to create one or more issues that represent these things so that we can address them down the line. If these things can be decided sooner rather than later and you want to work them out first thing, that's great too. |
3 participants
I don't think this breaks anything! I'm intending only to add the function SiloWriter::writeKullMesh to the SiloWriter class. But, since it's my first pull request, something will probably go wrong.