Skip to content

feat: Perform all mesh-doctor checks at once #100

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

Merged
merged 21 commits into from
Jun 23, 2025

Conversation

alexbenedicto
Copy link
Contributor

@alexbenedicto alexbenedicto commented Jun 5, 2025

Closes #94

The goal of this PR is to create 2 new features called all_checks and main_checks that will be able to call every mesh-doctor feature already implemented that only performed "quality" checking of the mesh such as collocated_nodes, element_volumes.
The current list of all "checks" available is the following:
[ collocated_nodes, element_volumes, non_conformal, self_intersecting_elements, supported_elements ].

So now, using this following line:

mesh-doctor -i mesh_to_analyze.vtu all_checks

Is equivalent (currently) to performing these checks successively:

mesh-doctor -i mesh_to_analyze.vtu collocated_nodes
mesh-doctor -i mesh_to_analyze.vtu element_volumes
mesh-doctor -i mesh_to_analyze.vtu non_conformal
mesh-doctor -i mesh_to_analyze.vtu self_intersecting_elements
mesh-doctor -i mesh_to_analyze.vtu supported_elements

If you want to only perform the fastest checks, using this following line:

mesh-doctor -i mesh_to_analyze.vtu main_checks

Is equivalent (currently) to performing these checks successively:

mesh-doctor -i mesh_to_analyze.vtu collocated_nodes
mesh-doctor -i mesh_to_analyze.vtu element_volumes
mesh-doctor -i mesh_to_analyze.vtu self_intersecting_elements

The code was designed in a way to allow any new future "check" action to be easily added.

Major changes

  1. While creating this PR, it felt appropriate to rename the folder "checks" by "actions" which is less ambiguous regarding the capabilities of mesh-doctor which can also perform operations on the mesh. The name "all_checks" makes now more sense.

  2. Previously, most of the "check" actions did not have default parameters to use them which is now the case + every one of their argparse keyword is now unique amongst all features. You cannot encounter two times the keyword "tolerance" in the "check" actions.

  3. import logging as now been replaced by the internal logging system configured in geos-utils which closes Improve mesh_doctor logging #41 .
    Add all parameters used as output in log.

  4. Automatic documentation for mesh-doctor instead of hard-coded command line bash.
    Update documentation.

Copy link
Contributor

@paloma-martinez paloma-martinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this work, it will be very useful.

Could you separate the different checks in the log ? It is a bit crowded, the different sections would appear more clearly with just an additional line break for example

Copy link

@dkachuma dkachuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't actually tried this but it looks good.

@paloma-martinez
Copy link
Contributor

The log modification looks good to me.

I agree with the removal of expensive checks from default behaviour, but I find the term all_checks a bit misleading now...
I'd like to suggest adding another option that actually performs all tests and rename the one that only does the 3 main checks, so that we would have something like:

  • main_checks that performs [ collocated_nodes, element_volumes, self_intersecting_elements ]
  • all_checks that does [ collocated_nodes, element_volumes, non_conformal, self_intersecting_elements, supported_elements ], maybe with a warning that it may be long

@alexbenedicto
Copy link
Contributor Author

@paloma-martinez Thank you for the feedback.
This can indeed be ambiguous and might need this new "main_checks" to be added as you suggest. It is only the code implementation that I need to think about to avoid maintaining two different mesh-doctor features "all_checks" and "main_checks" if the difference is only about what checks are called.

Copy link

@dkachuma dkachuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@alexbenedicto alexbenedicto merged commit c9b4b44 into main Jun 23, 2025
43 checks passed
@alexbenedicto alexbenedicto deleted the benedicto/feature/mesh-doctor-checks-in-one branch June 23, 2025 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Perform all checks of mesh-doctor in one command line Improve mesh_doctor logging
3 participants