Skip to content

Conversation

@stas00
Copy link
Contributor

@stas00 stas00 commented Aug 22, 2021

fixes a circular import problem introduced by #69 (comment)

___________________________________________ ERROR collecting Megatron-DeepSpeed-master/tests/test_preprocessing.py _____________________________________________
ImportError while importing test module '/mnt/nvme1/code/huggingface/Megatron-DeepSpeed-master/tests/test_preprocessing.py'.
Hint: make sure your test modules/packages have valid Python names.                                                                                              
Traceback:                                                                                                                                                       
/home/stas/anaconda3/envs/py38-pt19/lib/python3.8/importlib/__init__.py:127: in import_module                                                                    
    return _bootstrap._gcd_import(name[level:], package, level)                                                                                                  
tests/test_preprocessing.py:21: in <module>                                                                                                                      
    from megatron.testing_utils import (                                                                                                                         
megatron/__init__.py:28: in <module>                                                                                                                             
    from .global_vars import get_args                                                                                                                            
megatron/global_vars.py:25: in <module>
    from .arguments import parse_args
megatron/arguments.py:25: in <module>
    from megatron.model.glu_activations import GLU_ACTIVATIONS
megatron/model/__init__.py:18: in <module>
    from .distributed import DistributedDataParallel
megatron/model/distributed.py:22: in <module>
    from megatron import get_args
E   ImportError: cannot import name 'get_args' from partially initialized module 'megatron' (most likely due to a circular import) (/mnt/nvme1/code/huggingface/Megatron-DeepSpeed-master/megatron/__init__.py)

@stas00 stas00 merged commit 4255845 into main Aug 22, 2021
@stas00 stas00 deleted the 69-fix branch August 22, 2021 19:41
import deepspeed

from megatron.enums import PositionEmbeddingType
from megatron.model.glu_activations import GLU_ACTIVATIONS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the first time I've seen that, removing __init__ file in megatron/model would help I think.

Copy link
Contributor Author

@stas00 stas00 Aug 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will require adapting all these:

./pretrain_gpt.py:from megatron.model import GPTModel, GPTModelPipe
./megatron/training.py:from megatron.model import Float16Module
./megatron/training.py:from megatron.model import DistributedDataParallel as LocalDDP
./megatron/optimizer/__init__.py:from megatron.model import LayerNorm
./megatron/schedules.py:from megatron.model import DistributedDataParallel as LocalDDP
./megatron/schedules.py:from megatron.model import Float16Module
./megatron/text_generation_utils.py:from megatron.model import DistributedDataParallel as LocalDDP
./megatron/text_generation_utils.py:from megatron.model import Float16Module
./megatron/model/realm_model.py:from megatron.model import BertModel
./megatron/model/transformer.py:from megatron.model import LayerNorm
./megatron/model/bert_model.py:from megatron.model import LayerNorm
./megatron/model/gpt_model.py:from megatron.model import LayerNorm
./tasks/zeroshot_gpt/evaluate.py:from megatron.model import GPTModel
./tasks/zeroshot_gpt/evaluate.py:from megatron.model import DistributedDataParallel as LocalDDP
./tasks/zeroshot_gpt/evaluate.py:from megatron.model import Float16Module
./checkpoint-analysis.ipynb:    "from megatron.model import GPTModel\n",
./pretrain_bert.py:from megatron.model import BertModel
./pretrain_t5.py:from megatron.model import T5Model
./tools/generate_samples_gpt.py:from megatron.model import GPTModel

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm all for it if we continue running into this issue, if you or someone else would like to tackle that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can create an issue for this - should be trivial for anybody as this is all basic python.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create an issue for now I guess.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you doing it already, or if you haven't started, I will create it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done: #73

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I'll try to take 5 mins whenever I have time to do it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as I commented, the issue has already been created!

let's have some simple issues open for contributors - that way it's easier for them to ease in with contributing.

adammoody pushed a commit to adammoody/Megatron-DeepSpeed that referenced this pull request Oct 27, 2022
Co-authored-by: microsoft-github-policy-service[bot] <77245923+microsoft-github-policy-service[bot]@users.noreply.github.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.

3 participants