Skip to content

Fvmadapt docs update[WIP]#346

Open
wandadars wants to merge 27 commits into
EdwardALuke:devfrom
wandadars:fvmadapt_docs_update
Open

Fvmadapt docs update[WIP]#346
wandadars wants to merge 27 commits into
EdwardALuke:devfrom
wandadars:fvmadapt_docs_update

Conversation

@wandadars

Copy link
Copy Markdown
Collaborator

In no way is this in a state ready to be reviewed. Just putting it here for tracking purposes. Plus the GitHub GUI is nice for visually comparing diffs for changes made.

@wandadars wandadars added the refactor Used for reorganization of code to make it simpler or more consistent label Jun 13, 2026
@wandadars wandadars force-pushed the fvmadapt_docs_update branch from cca4ea3 to 81cd348 Compare June 13, 2026 06:45
@EdwardALuke

Copy link
Copy Markdown
Owner

Some cautions here. One, I see that you are adding new join operators for logical operators. I think this should really be broken out into a separate pull request and not tied up with FVMAdapt documentation effort. I could see these as being generally useful beyond mesh adaption.

Keep in mind that we have a FVMAdapt library components, a FVMAdapt module that is not loaded into the solver, but rather utilized in a separate rule database, and at some point in the future three will be a FVMAdapt solver module for interfacing with solvers. So we might want to be careful about how we name FVMAdapt.lh for example. What you are putting in there is used internally in the FVMAdapt module (not the solver interface part). So we may want to think about how to name things. I could see circumstances where it is confusing in terms of what we are actually loading in. But my feeling is that this file should not be user facing, so it might be better to place it in the source directory with the loci files defined in the module to prevent confusion.

I think we need to be cautious about AI generated documentation for these files. It is important that the documentation is validated in some way, and I think the best way would be for a person to actually grok the code. I can see some

For example, I noted this in prism.h I find the following comment:

  • @brief Prism refinement tree used by FVMAdapt.
  • @InGroup fvmadapt_elements
  • A prism has two general polygonal end faces and nfold quadrilateral side
  • faces. Current split codes create zero, two, nfold, or 2*nfold children
  • according to numChildren().
  • @warning Side-face orientation for child prisms is tracked through
  • faceOrient. It is part of the neighbor/face-ordering contract and should
  • be changed only with matching updates to prism extraction and merge helpers.
    */

At best I think this comment is misleading and confusing (as is the code). I think that we need to be careful of creating alot of garbage comments that will make it even harder to understand the code. Instead we need to think about how we can validate AI generated comments. It might be helpful to somehow mark these as unreliable AI generated comments. But I think it is very important that someone actually goes through and understands what the code is doing to verify the comments actually make sense. In this regard, I think it is fairly dangerous to just go in and generate documentation like a wildman creating a mess of confusing documentation that is too big a task to sort through. Instead we probably should focus on some small set of lowest level elements of the abstraction and build up from there where at each step we arrive at a point where someone actually understands the code that is being documented and can sign off on it. The code is a mess as it is, but doubling the amount of text we have to look through to understand it is probably going to be less than helpful. For example, right now I am not sure exactly what nfold is. Is it number-of-faces old? Is it number-of-folds? I know that some of the code is concerned with folded faces, that are quad faces that are twisted. Is it related to that? I think we need to establish some of these basics first before trying to put some junk comment on everything. In the early phase of refining a prism we break it into 6 hexahedra, and then it is hexahedra from there on down. At any rate, I could see alot of work that needs to be done just on this one comment alone, and that is probably the best way to break this work down. The comment here creates alot of questions that probably needs to be answered. Looking into those questions will probably lead to other questions until we get to the most fundamental ones that we need to grok before attempting to grok other things.

@wandadars

Copy link
Copy Markdown
Collaborator Author

I'm with you on the comments. I had gone through an iteration of having Codex update/add some docstrings, but those are lacking the clarity and insight that I would like to see for a final version of a docstring. And many of those doc .md files are just for me, to help me get even a slight handle on the general idea and nomenclatures used in the coding. There are many where I look at the comment, and it's just impenetrable references to low-level stuff that isn't giving any helpful insight.

I do prefer to try and retain any docstrings or comments that were already existing and just try to polish up the appearance. On my list is to revisit all the docstrings that were generated and compare with the original ones just to make sure insight wasn't lost.

With regards to the highest level understanding, one of the things that I am think might be helpful is if we have some diagrams about the things that are being discussed/manipulated in the module. The various cell types, the numbering schemes, etc. I use Inkscape often, so I can draw up any vector art svg files and embed them into whatever the final documentation is (for reference, what is existing now will be slashed to the bone by the end of the process, to hopefully make any examination much easier). I think the images might go a long way towards making things a little clearer, showing the different ways things can be split, etc.

In the end we should have a glossary of conceptual terms that are clearly shown or explained to the readers with regards to the framework/formalism that is used to describe things in this module.

@EdwardALuke

Copy link
Copy Markdown
Owner

I have sent you the original mesh adaptation paper that discusses the basic approach. You might be able to extract some useful figures from this.

Qiuhan who worked on the code has moved on to other things, but I could ask her some questions to see if she can recall anything. She has been away from the code for quite some time so she probably doesn't remember too much details.

A glossary would be useful, but we are probably going to have to arrive at it the hard way by kind of rebuilding the code from the bottom up validating along the way. Probably the first place to start would be digging into the refinement plan representation which is a tree that is compressed into a string of bytes. This refinement plan gives the recursive recipe for how to refine a cell. This tree is different for the different cell types. So perhaps we could start with something like the tetrahedra plan and work out and document exactly how it is represented and then how it is converted into a mesh. Then repeat for the other element types. Once we get that documented then we could move onto how to generate the cell plans given refinement goals.

@wandadars wandadars force-pushed the fvmadapt_docs_update branch from af9909b to 101bff8 Compare June 15, 2026 00:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor Used for reorganization of code to make it simpler or more consistent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants