-
Notifications
You must be signed in to change notification settings - Fork 12
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
Addition of coverage
tool to unit tests in workflow
#21
Merged
Merged
Changes from 9 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
f337fcf
Added orchestrated tests to workflow
fmalatino a137551
Linting
fmalatino 63ddb26
Adding FV3_DACEMODE variable to tests
fmalatino ebf24de
Added data download and environment variables to translate workflow
fmalatino d2cdc8b
Updated setup.py to point to updated NDSL
fmalatino cb94382
Updates to translate test to remove gt_cache between tests
fmalatino 6675f76
Changed setup.py back to track 2024.04.00 release of NDSL
fmalatino 5a3c1e7
Moving NDSL reference back to develop
fmalatino 918ff32
Merge branch 'develop' into update/workflow
fmalatino 5dced20
Increased number of omp threads for testing
fmalatino 4ae14e1
Adding in dace mode and loglevel environment variables for tests
fmalatino a74ee82
Addition of environment variables to have settings be explicit
fmalatino ecd8d2a
Changed translate tests in translate.yml to use 1 omp thread
fmalatino 0828d72
Changed reference to NDSL in setup.py to latest release version
fmalatino 10ed431
Added coverage package to testing workflow
fmalatino 453e6b7
Linting
fmalatino 614aa47
Re-linting
fmalatino 0f45171
Fixed translate tests to use one rank
fmalatino 9d3dc6b
Merged develop into update/workflow
fmalatino c670a1b
Removed translate.yml
fmalatino c8e244a
Updated minimum python version requirement in doc of setup.py
fmalatino File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ | |
] | ||
|
||
test_requirements = ["pytest", "pytest-subtests", "serialbox"] | ||
ndsl_requirements = ["ndsl @ git+https://github.com/NOAA-GFDL/NDSL.git@2024.04.00"] | ||
ndsl_requirements = ["ndsl @ git+https://github.com/NOAA-GFDL/NDSL.git@develop"] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bad reference version. |
||
develop_requirements = test_requirements + ndsl_requirements + ["pre-commit"] | ||
|
||
extras_requires = { | ||
|
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the bare minimum for orchestration. We should probably run some pySHiELD code and the full FVDynamics case as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As of commit
453e6b7
the omp threads have been rolled back to 1, the NDSL version is back at 2024.04.00, and addedcoverage
to the test suite. I timed the current test suite to take 5601.23 s (~93.35 mins). Would it make sense to add in the pySHiELD code and FVDynamics case in another PR?Coverage result:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
93 mins and we are only touching 33% of the code? That's... disappointing. Is the coverage report aggregating everything correctly since all the test are done one by one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, this is very slow, could you try using more openmp threads?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OpenMP won't help, the execution is not slow part, the build is. Frank restricted to one rank test (e.g. testing the data generated on rank 0 instead of the entire cube) and that should take care of the slowness of the test overall