Feature #3140 openmp apps pcp genvxmask #3147
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…o most MET tools via regridding.
… threads that actually are available.
… in the vx_regrid library to avoid duplication.
… using stl::vector's instead of manual memory management. This required introducing a DataPlaneArray::at(int) accessor function. Modify some library and application code to get things compiling.
…ike apply_mask(). Doing so produced a Grid-Stat runtime failure.
…n all GHA tests more efficiently.
… parallelized regridding actually runs well and produces the expected output.
…nto feature_3132_openmp_vx_util
…runtime segfault in the unit_regrid.xml unit test.
…es that include break statements.
…he compilation.
… the maximum number of available threads. ci-run-unit
…nce that triggers a failure when ncdiff is used to compute differences: libgomp: Invalid value for environment variable OMP_NUM_THREADS ... ci-run-unit
…te unit_met_test_scripts.xml to add missing expected output file names. ci-run-unit
…uction(+:n) to avoid race conditions updating the counter. ci-run-unit
…rQube code smells.
added 8 commits
May 1, 2025 20:37
… to add ci-run-unit to the commit message
…g computed to hopefully resolve differences, ci-run-unit
…ove erroneous variable listed, ci-run-unit
…tency, ci-run-unit
…pe of variables as much as possible, ensure no differences ci-run-unit
added 2 commits
May 8, 2025 16:23
…ecifier to this declaration.
8 tasks
JohnHalleyGotway
approved these changes
May 8, 2025
There was a problem hiding this comment.
@jprestop I approve of these changes. They look great!
Thanks for linking to your tests in runtime improvement. I confirmed that no diffs are flagged in the by the testing workflow and that the overall number of SonarQube code smells are reduced. I reviewed the code changes and they match what I was expecting.
| } // end for y | ||
| } // end for x | ||
| } // end of parallel | ||
|
|
||
| if(!put_nc_data_with_dims(&mask_var, mask_data.data(), grid.ny(), grid.nx())) { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request adds OpenMP parallelization to the MET application code for Pcp-Combine (7 parallelizations added) and Gen-Vx-Mask (12 parallelizations added). The
feature_3140_openmp_apps_pcp_genvxmaskbranch was created from thefeature_3132_openmp_vx_utilbranch (PR #3136).The changes for this PR include OpenMP parallelization and SonarQube code smell remediation. In particular, the following code smells were addressed:
Expected Differences
Do these changes introduce new tools, command line arguments, or configuration file options? [No]
If yes, please describe:
Do these changes modify the structure of existing or add new output data types (e.g. statistic line types or NetCDF variables)? [No]
If yes, please describe:
Pull Request Testing
Describe testing already performed for these changes:
Validation Testing:
Recommend testing for the reviewer(s) to perform, including the location of input datasets, and any additional instructions:
Please review code changes for accuracy and consistency.
Review the validation testing (links above) to see the speedup.
This branch is built on
senecaat/d1/projects/MET/MET_pull_requests/met-12.1.0/rc1/MET-feature_3140_openmp_apps_pcp_genvxmaskin case you want to do any testing.Do these changes include sufficient documentation updates, ensuring that no errors or warnings exist in the build of the documentation? [Yes]
Do these changes include sufficient testing updates? [Yes]
Will this PR result in changes to the MET test suite? [No]
If yes, describe the new output and/or changes to the existing output:
Will this PR result in changes to existing METplus Use Cases? [No]
If yes, create a new Update Truth METplus issue to describe them.
Do these changes introduce new SonarQube findings? [No]
If yes, please describe:
While some differences are flagged in "new code" the overall number of SonarQube code smells are reduced by 44 (from 17,114 for develop to 17,070 for this PR).
Please complete this pull request review by [20250515].
Pull Request Checklist
See the METplus Workflow for details.
Select: Reviewer(s) and Development issue
Select: Milestone as the version that will include these changes
Select: METplus-X.Y Support project for bugfix releases or MET-X.Y Development project for the next coordinated release