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

Fixed invalid indexing of scalar datasets, and updated deprecated datset names #3

Merged
merged 2 commits into from
Jul 8, 2023
Merged

Conversation

mlouis9
Copy link

@mlouis9 mlouis9 commented Jul 5, 2023

Updated dataset names to conform to VERA_4.3 naming conventions, and added additional logic to caching/uncaching to account for scalar datasets. The VERA_4.3 output file that these modifications were tested against (VERA progression problem: p9) can be found here.

@mlouis9 mlouis9 marked this pull request as ready for review July 5, 2023 15:30
@aarograh
Copy link

aarograh commented Jul 5, 2023

Thanks for fixing this @mlouis9 . For the Kitware developers, I can confirm that the dataset name changes are correct. VERA used to accept several forms of input for certain datasets; it still accepts various names but always changes them to a standardized form, which is what @mlouis9 has used here. This is necessary to use VERACore with newer versions of VERA.

For context: I'm the lead developer for VERA at ORNL now (https://www.ornl.gov/staff-profile/aaron-m-graham) and @mlouis9 explored VERACore as part of a summer internship project.

@patrickoleary
Copy link
Member

patrickoleary commented Jul 5, 2023 via email

@lefebvrera
Copy link

@mlouis9 please don't forget to address the coding standard items (# , empty line containing whitespace, etc).

@jourdain
Copy link
Collaborator

jourdain commented Jul 6, 2023

The CI is complaining for some formatting. Just run black on the source to fix it

black....................................................................Failed
- hook id: black
- exit code: 1

would reformat vera_core/app/core/vera_out_file.py

Oh no! 💥 💔 💥
1 file would be reformatted, 21 files would be left unchanged.

codespell................................................................Passed
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

vera_core/app/core/vera_out_file.py:92:1: W293 blank line contains whitespace
vera_core/app/core/vera_out_file.py:93:9: E2[65](https://github.com/Kitware/VERACore/actions/runs/5466149069/jobs/9951156419?pr=3#step:4:66) block comment should start with '# '

@mlouis9
Copy link
Author

mlouis9 commented Jul 7, 2023

That should've fixed the formatting issues. The pre-commit run passes on my end.

@patrickoleary
Copy link
Member

patrickoleary commented Jul 7, 2023 via email

@aarograh
Copy link

aarograh commented Jul 7, 2023

I responded by email, but just to close the loop on GitHub where @mlouis9 can see too: yes, no problems with using that updated dataset that @mlouis9 was using.

I'll let @mlouis9 confirm that this is ready to merge.

@mlouis9
Copy link
Author

mlouis9 commented Jul 8, 2023

Yep, I can confirm; everything is ready.

@patrickoleary patrickoleary merged commit 9d4054d into Kitware:master Jul 8, 2023
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.

5 participants