Skip to content

Bugfix/xdmf multiple grids#20980

Open
rrsettgast wants to merge 3 commits into
visit-dav:developfrom
rrsettgast:bugfix/xdmf_multiple_grids
Open

Bugfix/xdmf multiple grids#20980
rrsettgast wants to merge 3 commits into
visit-dav:developfrom
rrsettgast:bugfix/xdmf_multiple_grids

Conversation

@rrsettgast

@rrsettgast rrsettgast commented Jun 1, 2026

Copy link
Copy Markdown

Disclaimer: I had claude take a run at a review of the XDMF reader after the first commit which contains the real fix. All other commits are claude finding something and fixing it. If the visit team wants to drop the other commits because claude didn't understand context, that is no problem with me. The first commit is the Bugfix I was intending to fix.

Description

Fixes XDMF reader handling for files with multiple top-level grids, especially spatial collections with mesh-specific fields.

This updates the XDMF database plugin to treat multiple top-level grids as separate meshes, resolve variables by the owning grid name, avoid leaking fields between sibling grids, and guard against NULL/out-of-range grids instead of crashing. It also skips unsupported top-level grids cleanly, adds debug messages for ignored/unsupported grid cases, and fixes an XdmfGrid ownership leak in metadata population.

This also includes several macOS/build compatibility fixes:

  • Add Darwin 25 handling in build_visit.
  • Patch Qt 6 arm64 NEON runtime detection on macOS.
  • Improve Qt tools configure/build/install error handling.
  • Fix a CMake typo for app bundle resource properties.
  • Remove deprecated std::ptr_fun usage.
  • Simplify archive target Python commands.
  • Ignore local build artifacts.

Type of change

  • Bug fix~~
  • [ ] New feature
  • [ ] Documentation update
  • [ ] Other

How Has This Been Tested?

Added a regression test in src/test/tests/databases/xdmf.py that generates a self-contained XDMF file with two top-level spatial collections. The test verifies that:

  • both top-level grids appear as separate meshes,
  • scalar variables are associated with the correct mesh,
  • fields do not leak across sibling top-level grids,
  • plotting/querying the second mesh field returns the expected min/max value.

Also ran git diff develop...HEAD --check successfully.

Reminders:

  • Please follow the style guidelines of this project.
  • Please perform a self-review of your code before submitting a PR and asking others to review it.
  • Please assign reviewers (see VisIt's PR procedures for more information).

Checklist:

  • I have commented my code where applicable.~~
  • [ ] I have updated the release notes.
  • [ ] I have made corresponding changes to the documentation.
  • I have added debugging support to my changes.~~
  • I have added tests that prove my fix is effective or that my feature works.~~
  • I have confirmed new and existing unit tests pass locally with my changes.~~
  • [ ] I have added new baselines for any new tests to the repo.
  • I have NOT made any changes to protocol or public interfaces in an RC branch.~~

@biagas

biagas commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@rrsettgast Thank you much for the fixes. We would appreciate having just the Xdmf changes in this PR.
If you want to submit the other changes, a separate PR for those is apropos.

@rrsettgast rrsettgast force-pushed the bugfix/xdmf_multiple_grids branch from 460430e to adb41f9 Compare June 10, 2026 02:10
@markcmiller86

Copy link
Copy Markdown
Member

FWIW...the xdmf plugin changes here look fine to me.

The added test case is good too though my preference would be for that xdmf input test file to be added to xdmf_test_data.tar.xz instead of inlined in the xdmf.py test script.

The changes to data/CMakeLists.txt and bv_qt.sh stuff I think needs to be put in a separate PR. I noticed @rrsettgast did a force push of some changes here last week.

@JustinPrivitera JustinPrivitera left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If you could trim this down to just the XDMF changes then we can tackle some of the other fixes in another PR. Then we can get this merged. Thanks!

@JustinPrivitera

Copy link
Copy Markdown
Member

@markcmiller86 will take the XDMF fixes from this work and draft a new PR with them.

@rrsettgast

Copy link
Copy Markdown
Author

Sorry. been busy. This should be done, although I can no longer build so I can't test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants