Skip to content

Conversation

@ndodda-amazon
Copy link
Contributor

@ndodda-amazon ndodda-amazon commented Jan 16, 2021

Reverts #411. #411 reintroduced two different bugs:

  1. Importing smdataparallel without conditioning on check_smdataparallel_env()
  2. Importing smdataparallel globally

The primary issue is that check_smdataparallel_env must be checked first before attempting to import. Attempting to import smdataparallel without the environment variable being set and being initialized via MPI will trigger failures in any non-SMP jobs. This has caused multiple failures in our profiler integration tests.

The solution is straightforward - move the imports back to get_distributed_worker and only attempt to import if check_smdataparallel_env is True.

TODO: Introduce profiler integration tests as a part of the CI to prevent this kind of issue from happening again.

@codecov-io
Copy link

codecov-io commented Jan 16, 2021

Codecov Report

Merging #424 (12fe775) into master (c6554a7) will decrease coverage by 36.99%.
The diff coverage is 39.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #424       +/-   ##
===========================================
- Coverage   77.95%   40.95%   -37.00%     
===========================================
  Files         113      113               
  Lines       10169    10143       -26     
===========================================
- Hits         7927     4154     -3773     
- Misses       2242     5989     +3747     
Impacted Files Coverage Δ
smdebug/tensorflow/base_hook.py 0.00% <0.00%> (-74.28%) ⬇️
smdebug/core/utils.py 76.27% <42.30%> (-3.45%) ⬇️
smdebug/mxnet/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
smdebug/pytorch/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
smdebug/tensorflow/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
smdebug/tensorflow/constants.py 0.00% <0.00%> (-100.00%) ⬇️
smdebug/mxnet/singleton_utils.py 0.00% <0.00%> (-100.00%) ⬇️
smdebug/pytorch/singleton_utils.py 0.00% <0.00%> (-100.00%) ⬇️
smdebug/tensorflow/singleton_utils.py 0.00% <0.00%> (-100.00%) ⬇️
...debug/profiler/analysis/notebook_utils/__init__.py 0.00% <0.00%> (-100.00%) ⬇️
... and 67 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 c6554a7...12fe775. Read the comment docs.

@ndodda-amazon ndodda-amazon merged commit e431609 into master Jan 16, 2021
@ndodda-amazon ndodda-amazon deleted the revert-411-smp branch January 16, 2021 00:45
NihalHarish added a commit that referenced this pull request Jan 25, 2021
This reverts commit 07a3fd9.

Co-authored-by: Nihal Harish <nihal42harish@gmail.com>
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.

4 participants