Skip to content
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

Adjust reader to use point count rather than storage size in size check #19530

Merged
merged 6 commits into from
May 20, 2024

Conversation

markcmiller86
Copy link
Member

@markcmiller86 markcmiller86 commented May 17, 2024

Description

See #19514.

This same storage size vs. point count logic occurs in multiple places in the reader and so I fixed it in multiple places. I also encountered cases where datasetStorageSize was being set but never used and cases where HDF5 objects were being opened but the coorresponding H5Xclose() calls were commented out. I uncommented them.

Resolves #19529

Type of change

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

How Has This Been Tested?

I tested as a custom plugin against 3.3.1 and opened both the one test case we have and also the data the user attached to the above ref'd discussion. It all worked.

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.~~

@@ -25,6 +25,7 @@
<li>Fixed bug where user supplied legend labels were stored in config and session files wrapped in '()' and comma separated. This prevented the values from being parsed correctly and the '()' and commas appeared as part of the labels.</li>
<li>The location where custom plugins are written when built against installed version of VisIt was corrected.</li>
<li>Windows build issues for custom plugins against an installed version of VisIt was fixed.</li>
<li>Fixed a bug in OpenPMD reader that subvertted its ability to read chunked, as opposed to contiguous datasets.</li>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<li>Fixed a bug in OpenPMD reader that subvertted its ability to read chunked, as opposed to contiguous datasets.</li>
<li>Fixed a bug in OpenPMD reader that subverted its ability to read chunked, as opposed to contiguous datasets.</li>

Copy link
Contributor

@biagas biagas left a comment

Choose a reason for hiding this comment

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

Any reason to not run the test yourself locally, rather than waiting to see if the nightly fails after this commit?

src/databases/OpenPMD/OpenPMDClasses/PMDFile.C Outdated Show resolved Hide resolved
src/resources/help/en_US/relnotes3.4.2.html Outdated Show resolved Hide resolved
@markcmiller86
Copy link
Member Author

Any reason to not run the test yourself locally, rather than waiting to see if the nightly fails after this commit?

Ok, testing now. Also found another couple of places where that bad size logic was being used and corrected it.

Thanks for the spelling corrections 💪🏻.

@markcmiller86
Copy link
Member Author

Any reason to not run the test yourself locally, rather than waiting to see if the nightly fails after this commit?

Yes...doing too many things at once. My VisIt installs are hosed for the moment. But, I did build the plugin as a custom plugin against 3.3.1 and tested on both our existing openpmd test data and on the user's data. Works for both!.

Is that sufficient?

@markcmiller86 markcmiller86 requested a review from biagas May 20, 2024 18:07
@markcmiller86 markcmiller86 merged commit 13983eb into 3.4RC May 20, 2024
4 checks passed
markcmiller86 added a commit that referenced this pull request May 20, 2024
…ck (#19530)

* use logical size for size check

* Update src/databases/OpenPMD/OpenPMDClasses/PMDFile.C

Co-authored-by: Kathleen Biagas <[email protected]>

* Update src/resources/help/en_US/relnotes3.4.2.html

Co-authored-by: Kathleen Biagas <[email protected]>

* rm unused var

* fix size checking logic

---------

Co-authored-by: Kathleen Biagas <[email protected]>
@markcmiller86 markcmiller86 deleted the bug-mcm86-17may24-openpmd-sizeck branch May 20, 2024 19:43
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.

None yet

3 participants