Closed ndodda-amazon closed 3 years ago
Merging #429 (7b00751) into master (8455a64) will increase coverage by
0.60%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## master #429 +/- ##
==========================================
+ Coverage 76.99% 77.60% +0.60%
==========================================
Files 113 113
Lines 10171 10171
==========================================
+ Hits 7831 7893 +62
+ Misses 2340 2278 -62
Impacted Files | Coverage Δ | |
---|---|---|
smdebug/core/reduction_config.py | 88.31% <0.00%> (-7.80%) |
:arrow_down: |
smdebug/core/singleton_utils.py | 85.29% <0.00%> (-5.89%) |
:arrow_down: |
smdebug/core/tfevent/util.py | 92.00% <0.00%> (-4.00%) |
:arrow_down: |
smdebug/pytorch/hook.py | 76.03% <0.00%> (-0.64%) |
:arrow_down: |
smdebug/core/tfevent/summary.py | 88.13% <0.00%> (ø) |
|
smdebug/core/save_config.py | 88.80% <0.00%> (+0.79%) |
:arrow_up: |
smdebug/core/tfevent/timeline_file_writer.py | 97.05% <0.00%> (+0.98%) |
:arrow_up: |
smdebug/core/logger.py | 74.19% <0.00%> (+1.61%) |
:arrow_up: |
smdebug/exceptions.py | 69.04% <0.00%> (+3.57%) |
:arrow_up: |
smdebug/core/reader.py | 88.88% <0.00%> (+3.70%) |
:arrow_up: |
... and 9 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 8455a64...eda5ff7. Read the comment docs.
Can you add enable the integration test codebuild job as part of this PR?
Can you add enable the integration test codebuild job as part of this PR?
Added. As you can see, the TF 2.3 and PT 1.6 builds pass trivially because they skip the integration tests (only files in the config
folder are modified in this PR)
Let's see how we can reduce code duplication across YAML files.
See https://stackoverflow.com/questions/14184971/more-complex-inheritance-in-yaml
See https://stackoverflow.com/questions/14184971/more-complex-inheritance-in-yaml
As discussed offline, this solution won't work since we want to have separate buildspecs for each build. However, we can cut down on the duplicated code by moving all of the commands to a shell script and only running that shell script within the buildspec.
Also moved the dependent packages to a requirements.txt, which will be easy to update moving forward. We should probably take the same approach in all of our other repos.
Description of changes:
This script is used to conditionally run profiler integration tests in the CI in two ways:
disable_integration_tests
environment variable in this script so that profiler integration tests are not run in the CI. This meant to be a temporary tool for speeding up development - the environment variable must be reset to"true"
before approving and merging a PR.See this PR for more details, which is where this script will be used.
Also cut down on code duplication by:
requirements.txt
so there's a single file that manages packages.Style and formatting:
I have run
pre-commit install
to ensure that auto-formatting happens with every commit.Issue number, if available
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.