Refactored code and small changes #21

ma7dev · 2022-04-29T14:38:14Z

Restructured the codebase as mentioned in the issue [Proposal] Codebase refactoring #18
Unexpected changes:
- Missing packages to run demo.ipynb:
  - jupyter
  - inflect
  - matplotlib
  - gdown
- Change of hardcoded paths in FastSpeech2 to be passed paths to the
  module.
- Removed inference.py as it doesn't correlate to anything actually
  being used in the library.

- Restructured the codebase as mentioned in the issue ARBML#18 - Unexpected changes: - Missing packages to run demo.ipynb: - jupyter - inflect - matplotlib - gdown - Change of hardcoded paths in FastSpeech2 to be passed paths to the module. - Removed inference.py as it doesn't correlate to anything actually being used in the library.

MagedSaeed · 2022-04-30T01:42:21Z

pyproject.toml

+
+[tool.poetry.dependencies]
+python = "^3.8"
+rich = "^12.2.0"


I do not think we are using rich in klaam here, are we?

I just included rich because it makes the output nicer :p there isn't any harm for that. I can remove it.

MagedSaeed · 2022-04-30T01:44:32Z

pyproject.toml

+mishkal = "^0.4.1"
+codernitydb3 = "^0.6.0"
+pypinyin = "^0.46.0"
+transformers = "^4.18.0"


I am not much familiar with poetry and pyproject.toml syntax. However, if '^' means this version or higher, then I do not think this is recommended. This is because backward compatibility is not guaranteed in such highly dynamic packages. I run into these issues before, one of these issues was with jiwer package :(.

You are correct ^ means this version or higher, however, when it installs it will only install that exact version, not the latest version possible (to my knowledge). However, this tag is helpful when you have a new package that you want to install but it requires a high version of your already installed package, so poetry would install the new package and update the old package with a new version.

For example, I have installed numpy and got this version numpy = "^0.1.0", then I wanted to install pandas which requires numpy >= 0.5.0 so poetry will update numpy to be numpy = "^0.5.0" and then install pandas.

You can restrict the versioning of packages in two ways:

When installing the package, explicitly state the version of the package. For example, poetry add numpy=0.1.0 will install numpy with 0.1.0 then append/update pyproject.toml to be numpy = "0.1.0".

Editing pyproject.toml to remove ^ from the packages then reinstall the packages.

Both methods will install a specific version of numpy but for any new library that requires a different version, poetry will scream at you to tell you that your project has numpy=0.1.0 but the newer packages require a different version of numpy.

I think we should stick with your approach @sudomaze . Thanks for the details.

MagedSaeed · 2022-04-30T01:45:54Z

pyproject.toml

+pytest = "^7.1.1"
+pylint = "^2.13.5"
+autopep8 = "^1.6.0"
+gdown = "^4.4.0"


I think we need this to download libraries from google drive. Should it be in the requirements section instead of dev_requirements?

I agree with you. I will move it to dependencies rather than dev-dependencies.

MagedSaeed · 2022-04-30T01:46:38Z

pyproject.toml

+pylint = "^2.13.5"
+autopep8 = "^1.6.0"
+gdown = "^4.4.0"
+pandas = "1.3.5"


I think pandas,tqdm should be in dependencies instead of dev-dependencies.

I agree with you. I will move it to dependencies rather than dev-dependencies.

MagedSaeed · 2022-04-30T01:47:52Z

pyproject.toml

+readme = 'README.md'
+
+[tool.poetry.dependencies]
+python = "^3.8"


@zaidalyafeai , I think most of the code runs on google colab. Google colab currently runs on python3.7 right?

I couldn't run the project using python 3.7.13 as some packages require new versions. Google Colab does use python 3.7.13, maybe we do need to resolve some conflicts with packages to be able to use python 3.7.13.

If Python3.8 is covered by most LTS Linux OSes ( I am assuming users will have Linux LTS in their machines) then this should be fine. Debian 11 supports Python3.9, Ubuntu 20.04 supports Python 3.8. I think it should be fine to stick with Python 3.8 and onwards. Google colab will update soon

MagedSaeed · 2022-04-30T01:49:10Z

.envs/environment.yml

+  - ffmpeg
+  - cudatoolkit
+  - pytorch
+  - torchaudio


I think soundfile package is missing?

I think soundfile is a dependency of another package (maybe torchaudio) that is installed by default. I have managed to run notebooks/demo.ipynb without installing soundfile explicitly.

To give more context, .env/dev_environment.yml contains the same packages as .env/environment.yml but CPU version so we can run CI on GitHub servers.

You are right about soundfile [https://pytorch.org/audio/stable/backend.html]

MagedSaeed · 2022-04-30T01:50:21Z

.envs/environment.yml

+  - python=3.8
+  - poetry
+  # klaam dependencies
+  - librosa


I am really confused which should be considered as a dev_environment vs environmlent and the choices of packages for each.

To give more context, .env/dev_environment.yml contains the same packages as .env/environment.yml but CPU version so we can run CI on GitHub servers.

What exists in environment.yml are the core packages that are heavy to be installed, so we use conda to install them while setting up the environment.

ckpts/.gitignore

MagedSaeed · 2022-04-30T01:52:20Z

notebooks/.gitignore

@@ -0,0 +1,3 @@
+*
+!*.ipynb
+!.gitignore


Do we really to have a .gitignore for each subdirectory?

The main .gitignore on the root directory is used to ignore everything globally (i.e. what is in klaam directory, scripts directory, and other directly related directories to the logic code like the data directory). As notebooks is an external module that isn't a logic component but a different type of scripts, but for demoing. Here is my logic for having multiple .gitignore:

.gitignore = global things to be ignored and what is in logic code (i.e. klaam).

notebooks/.gitignore = ignores what is not ipynb

ckpts/.gitignore = ignores everything besides the .gitignore file. We don't GitHub to remove this folder when we push the code to GitHub servers.

output/.gitignore = same thing as ckpts/.gitignore

MagedSaeed · 2022-04-30T01:54:08Z

install.sh

+#!/bin/bash
+# Usage: ./install.sh  # klaam
+# Usage: ./install.sh --dev # klaam_dev
+


Can you please let me know what this bash code is doing? is it setting the conda environment whether it is dev or not?

This is just a simple script that you run to do the following:

Build the environment using conda

Install packages using poetry

Install left-over packages that couldn't be installed using poetry using pip

./install.sh installs the actual environment of klaam that uses GPU, whereas ./install.sh --dev installs the CPU version of the actual environment called klaam_dev to be able to test the build and run it on GitHub servers through GitHub Actions (CI).

I see. Thanks.

MagedSaeed · 2022-04-30T01:55:45Z

data/model_weights/hifigan/LICENSE

+AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+SOFTWARE.


I think this file is added here by mistake?

I have copied this file to where the content should exist. This was part of the FastSpeech2 directory (https://github.com/ARBML/klaam/tree/main/FastSpeech2/hifigan).

As we restructured the setup of the project, FastSpeech2 contained a lot of hard-coded relative paths to FastSpeech2 project, which isn't a good practice and won't allow the current setup to work unless we do some dirty hacks which could cause more problems in later developments of the project.

Therefore, I needed to move configs and weights files to be outside of the logic code. To follow the license information, I have copied the LICENSE file where I have moved the configs and weights files.

This can be removed if you think it isn't necessary.

Before investing anything related to FastSpeech, I think we should remove it @zaidalyafeai please comment on this.

MagedSaeed · 2022-04-30T01:58:04Z

klaam/__init__.py

@@ -0,0 +1 @@
+from klaam.run import *


I really disagree with the wild card imports as it results in a lot of name space conflicts. I think from klaam.run import SpeechRecognition SpeechClassification TextToSpeech should nail it?

I agree with you. I will update this to be more explicit to select modules.

MagedSaeed · 2022-04-30T02:05:26Z

.github/workflows/ci.yml

+  push:
+    branches:
+      - main
+      - dev/*


@sudomaze, do you think we should have a stable branch that deploy to pypi directoy on merge or main is enough? I think we should also maintain another testing branch for test.pypi. What do you think?

The idea that I had in mind was to publish to pypi through GitHub Releases triggers, so we will have a GitHub Action of publishing only when something is released. This will limit any mistakes of publishing bad versions and this disconnects development code and stable releases.

We can add a nightly version of the package which pulls from a stable branch (i.e. nightly-release branch).

I see! so only one main branch is enought.

MagedSaeed · 2022-04-30T02:10:52Z

Thanks @sudomaze for this great PR.. I did my best commenting on points I did not understand/disagree.

Thanks again.

ma7dev · 2022-04-30T06:45:47Z

@MagedSaeed thanks for the thorough review! I will make the required changes when I have some time in the upcoming days.

I have updated TextToSpeech module to fix the hardcoded relative paths (mentioned above) and I have found that this approach, shown in notebooks/demo.ipynb, is the easiest approach without modifying the config files and changing a lot of the FastSpeech2. As you were suggesting the removal of this module, so I just made this update that made sense to make to resolve the issue while not investing a lot of time to refactor FastSpeech2's main codebase. What are your thoughts?

@MagedSaeed

- Made the suggested changes by @MagedSaeed: - Removed `rich` in `pyproject.toml` - Moved `gdown` and `pandas` from `dev-dependencies` to `dependencies` in `pyproject.toml` - Re-built the environment and made the required changes to be able to use `python=3.7.13`. The issue I have had previously when setting up the environment was due to `pandas` installation. It seems that when adding `python="^3.7"` in `pyproject.toml` then installing a new package, `poetry` will try to ensure that the being installed package worked on all versions of `python` that is `>=3.7`. However, in the newer version of `pandas`, some modules changed drastically that it breaks when `python` is `3.8>=` vs. `>=3.8`. This also solved the problem of `numba` not being able to be installed using `poetry`. For more information, check [this issue](https://togithub.com/python-poetry/poetry/issues/2454#issuecomment-632868431). [^1] - Made additional changes to improve the setup: - As mentioned in [^1], fixing this issue allowed us to move all of the packages, that were included in `.envs/environment.yml` (e.g. `pytorch`, `torchaudio`, etc.), to `pyproject.toml` as we don't have the package installation issue. - Moved `mpy.ini` and `pytest.ini` configuration to `pyproject.toml` to centerlize the project's configurations. - Changed to `black` instead of `autopep8`. For more information, check [this article](https://www.kevinpeters.net/auto-formatters-for-python). - Fixed hard-coded paths in `cfgs/FastSpeech2/config/Arabic/preprocess.yaml` by passing a variable called `root_path`, which is the relative/abosulte of the root directory to `TextToSpeech` class. (TODO: need to test the setup on Google Colab) - Added `pre-commit` to the build setup which allows to check the format, auto-format, and other useful functionalities before making a commit. - Publishing attempt: - I have tried to publish the package to `PyPi` to test if the package is modulized enough that I can `pip install` it. Because of the size limitation (`100MB` max), I needed to remove the weights and config file in `klaam/external/FastSpeech2/hifigan`. - Managed to publish the package! Package [page](https://pypi.org/project/klaam/). This is just a test, so I will be removing this package once we have setup the CI for it as we have discussed before.

ma7dev · 2022-04-30T14:27:57Z

The latest commit's message:

Made the suggested changes by @MagedSaeed:
- Removed rich in pyproject.toml
- Moved gdown and pandas from dev-dependencies to dependencies in pyproject.toml
- Re-built the environment and made the required changes to be able to use python=3.7.13. The issue I have had previously when setting up the environment was due to pandas installation. It seems that when adding python="^3.7" in pyproject.toml then installing a new package, poetry will try to ensure that the being installed package worked on all versions of python that is >=3.7. However, in the newer version of pandas, some modules changed drastically that it breaks when python is 3.8>= vs. >=3.8. This also solved the problem of numba not being able to be installed using poetry. For more information, check this issue. [^1]
Made additional changes to improve the setup:
- As mentioned in [^1], fixing this issue allowed us to move all of the packages, that were included in .envs/environment.yml (e.g. pytorch, torchaudio, etc.), to pyproject.toml as we don't have the package installation issue.
- Moved mpy.ini and pytest.ini configuration to pyproject.toml to centerlize the project's configurations.
- Changed to black instead of autopep8. For more information, check this article.
- Fixed hard-coded paths in cfgs/FastSpeech2/config/Arabic/preprocess.yaml by passing a variable called root_path, which is the relative/abosulte of the root directory to TextToSpeech class. (TODO: need to test the setup on Google Colab)
- Added pre-commit to the build setup which allows to check the format, auto-format, and other useful functionalities before making a commit.
Publishing attempt:
- I have tried to publish the package to PyPi to test if the package is modulized enough that I can pip install it. Because of the size limitation (100MB max), I needed to remove the weights and config file in klaam/external/FastSpeech2/hifigan.
- Managed to publish the package! Package page. This is just a test, so I will be removing this package once we have setup the CI for it as we have discussed before.

Formated the code using black and other pre-commit-hooks. Sorry for the huge number of changes, it was due to this.

ma7dev · 2022-05-01T11:12:11Z

Managed to make a CI to publish to PyPI when pushing to a stable/* branch where the naming means that any branch that starts with stable/ will run the CI. Job Status for test on test_stable/* to publish to PyPI test

MagedSaeed · 2022-05-05T02:00:47Z

Thanks @sudomaze for the updates.

I just updated to your PR, tried to run poetry update, then poetry install or pip install -e ., then check the demo file and everything works as expected.

Just a couple of things before merging to main.

I only noticed that it really takes some time to resolve dependencies. Is this usual with poetry:?

I also found out that if the operation is interrupted, I need to remove poetry cache in order for things to seamlessly work again. Do I need to disable caching here? I think there should be a flag to install from remote or at least to update cache, right?

I tried to run the project on Python3.7 as you mentioned, but this fails regarding to a conflict in numba. Also, the project did not start until I manually set it to python3.8 as it is on my system. Do you think we should have ^3.7.13 if klaam is going to support python3.7?

Once again, thanks much for this great effort.

ma7dev · 2022-05-05T10:21:58Z

I only noticed that it really takes some time to resolve dependencies. Is this usual with poetry:?

Yes, because we have a lot of dependencies and some of them, like torch, have a lot of dependencies, so it is normal. If we use conda to install the packages, we will have the same behavior.

I also found out that if the operation is interrupted, I need to remove poetry cache in order for things to seamlessly work again. Do I need to disable caching here? I think there should be a flag to install from remote or at least to update cache, right?

I would assume by cache as poetry.lock file, right? If so, poetry needs this file to know the hashing of the packages that were installed. You will need to remove poetry.lock when you have drastic changes in your pyproject.toml file or you want to re-install all of the packages using poetry install command.

I tried to run the project on Python3.7 as you mentioned, but this fails regarding to a conflict in numba. Also, the project did not start until I manually set it to python3.8 as it is on my system. Do you think we should have ^3.7.13 if klaam is going to support python3.7?

If you pull my latest changes, you should be able to install numba through poetry when python="3.7.13". I think we can have it to be python=3.7.13 only until we remove other un-required parts, like FastSpeech2, then we can re-organize the packages and upgrade python version to the latest and test on different python versions. In other words, let's keep it python="3.7.13" so you can run things through Google Colab.

MagedSaeed · 2022-05-11T03:27:27Z

Sorry for being a bit late @sudomaze . Got quite busy these days.

I tried to run the training scripts but found some issues regarding the models and preprocessors import like:

Traceback (most recent call last):
  File "scripts/run_classifier.py", line 15, in <module>
    from processors import CustomWav2Vec2Processor
ModuleNotFoundError: No module named 'processors'

I think I need to import these classes in __init__.py of models and processors folders and change the import statement to from klaam.processors import ...? I tried this and workd. I think these are the two folders needed to import. I did not catch any others. Can you please double-check?

ma7dev · 2022-05-11T04:19:00Z

Can you share or make some scripts that you think should work with the old setup? I am basing the success of my implementation on the demo.ipynb notebook.

zaidalyafeai · 2022-05-11T04:29:15Z

The training scripts in the readme file.

ma7dev · 2022-05-11T05:23:12Z

@zaidalyafeai

On main branch codebase, I have tried the following

run_classifier.py

Run script

python run_classifier.py \
>     --model_name_or_path="facebook/wav2vec2-large-xlsr-53" \
>     --output_dir=dialects-classificationv3 \
>     --cache_dir=cache\
>     --freeze_feature_extractor \
>     --num_train_epochs="5" \
>     --per_device_train_batch_size="16" \
>     --preprocessing_num_workers="1" \
>     --learning_rate="3e-5" \
>     --warmup_steps="20" \
>     --evaluation_strategy="steps"\
>     --save_steps="1000" \
>     --eval_steps="1000" \
>     --save_total_limit="1" \
>     --logging_steps="1000" \
>     --do_eval \
>     --do_train \

Error message

Traceback (most recent call last):
  File "run_classifier.py", line 440, in 
    main()
  File "run_classifier.py", line 256, in main
    parser = HfArgumentParser((ModelArguments, DataTrainingArguments, TrainingArguments))
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/hf_argparser.py", line 71, in __init__
    self._add_dataclass_arguments(dtype)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/hf_argparser.py", line 166, in _add_dataclass_arguments
    self._parse_dataclass_field(parser, field)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/hf_argparser.py", line 137, in _parse_dataclass_field
    parser.add_argument(field_name, **kwargs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/argparse.py", line 1373, in add_argument
    return self._add_action(action)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/argparse.py", line 1736, in _add_action
    self._optionals._add_action(action)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/argparse.py", line 1577, in _add_action
    action = super(_ArgumentGroup, self)._add_action(action)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/argparse.py", line 1387, in _add_action
    self._check_conflict(action)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/argparse.py", line 1526, in _check_conflict
    conflict_handler(action, confl_optionals)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/argparse.py", line 1535, in _handle_conflict_error
    raise ArgumentError(action, message % conflict_string)
argparse.ArgumentError: argument --gradient_checkpointing: conflicting option string: --gradient_checkpointing

run_mgb3.py

Run script

python run_mgb3.py \
>     --model_name_or_path="facebook/wav2vec2-large-xlsr-53" \
>     --output_dir=dialects-classificationv3 \
>     --cache_dir=data_dir \
>     --freeze_feature_extractor \
>     --num_train_epochs="50" \
>     --per_device_train_batch_size="32" \
>     --preprocessing_num_workers="1" \
>     --learning_rate="3e-5" \
>     --warmup_steps="20" \
>     --evaluation_strategy="steps"\
>     --save_steps="100" \
>     --eval_steps="100" \
>     --save_total_limit="1" \
>     --logging_steps="100" \
>     --do_eval \
>     --do_train

Error message

...
Downloading and preparing dataset egyption_speech_corpus/clean to data_dir/egyption_speech_corpus/clean/2.1.0/1d7beb58f0291b97069c539ff2a7e0d49e3fb3993a6d88415956acfd01e22911...
Traceback (most recent call last):                      
  File "run_mgb3.py", line 523, in 
    main()
  File "run_mgb3.py", line 308, in main
    train_dataset = datasets.load_dataset("egy_speech_corpus", split='train', cache_dir=model_args.cache_dir)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/datasets/load.py", line 1696, in load_dataset
    use_auth_token=use_auth_token,
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/datasets/builder.py", line 606, in download_and_prepare
    dl_manager=dl_manager, verify_infos=verify_infos, **download_and_prepare_kwargs
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/datasets/builder.py", line 1104, in _download_and_prepare
    super()._download_and_prepare(dl_manager, verify_infos, check_duplicate_keys=verify_infos)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/datasets/builder.py", line 701, in _download_and_prepare
    ) from None
OSError: Cannot find data file. 
Original error:
[Errno 2] No such file or directory: '/content/mgb3/adapt/Alaa/text_noverlap'

run_common_voice.py

Run script

python run_common_voice.py \
>     --model_name_or_path="facebook/wav2vec2-large-xlsr-53" \
>     --dataset_config_name="ar" \
>     --output_dir=dialects-classificationv3 \
>     --cache_dir=data_dir \
>     --overwrite_output_dir \
>     --num_train_epochs="1" \
>     --per_device_train_batch_size="32" \
>     --per_device_eval_batch_size="32" \
>     --evaluation_strategy="steps" \
>     --learning_rate="3e-4" \
>     --warmup_steps="500" \
>     --fp16 \
>     --freeze_feature_extractor \
>     --save_steps="10" \
>     --eval_steps="10" \
>     --save_total_limit="1" \
>     --logging_steps="10" \
>     --group_by_length \
>     --feat_proj_dropout="0.0" \
>     --layerdrop="0.1" \
>     --gradient_checkpointing \
>     --do_train --do_eval \
>     --max_train_samples 100 --max_val_samples 100

Error message

...
Traceback (most recent call last):
  File "run_common_voice.py", line 511, in 
    main()
  File "run_common_voice.py", line 479, in main
    train_result = trainer.train(resume_from_checkpoint=checkpoint)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/trainer.py", line 1422, in train
    tr_loss_step = self.training_step(model, inputs)
  File "run_common_voice.py", line 230, in training_step
    loss = self.compute_loss(model, inputs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/trainer.py", line 2043, in compute_loss
    outputs = model(**inputs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/modules/module.py", line 1110, in _call_impl
    return forward_call(*input, **kwargs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/models/wav2vec2/modeling_wav2vec2.py", line 1721, in forward
    return_dict=return_dict,
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/modules/module.py", line 1110, in _call_impl
    return forward_call(*input, **kwargs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/models/wav2vec2/modeling_wav2vec2.py", line 1347, in forward
    extract_features = self.feature_extractor(input_values)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/modules/module.py", line 1110, in _call_impl
    return forward_call(*input, **kwargs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/models/wav2vec2/modeling_wav2vec2.py", line 515, in forward
    hidden_states = conv_layer(hidden_states)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/modules/module.py", line 1110, in _call_impl
    return forward_call(*input, **kwargs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/transformers/models/wav2vec2/modeling_wav2vec2.py", line 390, in forward
    hidden_states = self.layer_norm(hidden_states)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/modules/module.py", line 1110, in _call_impl
    return forward_call(*input, **kwargs)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/modules/normalization.py", line 190, in forward
    input, self.normalized_shape, self.weight, self.bias, self.eps)
  File "/home/alotaima/miniconda3/envs/klaam/lib/python3.7/site-packages/torch/nn/functional.py", line 2486, in layer_norm
    return torch.layer_norm(input, normalized_shape, weight, bias, eps, torch.backends.cudnn.enabled)
RuntimeError: CUDA out of memory. Tried to allocate 1.38 GiB (GPU 0; 10.75 GiB total capacity; 5.85 GiB already allocated; 406.06 MiB free; 8.04 GiB reserved in total by PyTorch) If reserved memory is >> allocated memory try setting max_split_size_mb to avoid fragmentation.  See documentation for Memory Management and PYTORCH_CUDA_ALLOC_CONF

My thoughts on the errors:

run_classifier.py seems to have conflict in passed arguments.
run_mgb3.py seems to have hard coded paths.
run_common_voice.py needs more memory...

Have you tested the code locally? Or does the current codebase only works on Google Colab's notebooks?

zaidalyafeai · 2022-05-11T06:00:47Z

No only on colab.

ma7dev · 2022-05-11T06:04:07Z

Don't you think it is a problem? This requires more refactoring 😫

ma7dev · 2022-05-11T06:07:40Z

@MagedSaeed it should be the following

klaam.processors.wav2vec import CustomWav2Vec2Processor, it works. As for adding them to __init__.py it isn't a good practice as you will have a lot of modules in the future to be added to the processors.

MagedSaeed · 2022-05-11T06:29:00Z

Don't you think it is a problem? This requires more refactoring tired_face

I think this will also happen on colab as well if the later version of transformers is installed. I remember I stumbled into this error before. If I recall correctly, this is because this flag was introduced in early versions of the package. In later versions, they removed it, I guess.

ma7dev · 2022-05-11T06:47:29Z

@zaidalyafeai @MagedSaeed
I guess another PR is required to update the dead code? I don't think it is good to fix problems in the original code for this PR as this PR is for refactoring only.

If you prefer, we can set up a call to discuss project plans and what to do. What do you think?

zaidalyafeai · 2022-05-11T20:10:59Z

I'm on travel. If Maged can meet go ahead.

MagedSaeed · 2022-05-14T18:29:23Z

It turns out that this issue has been resolved in #19 . I think you forked an older version of the code before this fix happened. Can you make sure to include these latest changes, please?

ma7dev · 2022-05-14T18:44:24Z

@MagedSaeed this resolves one problem out of multiple problems caused by hardcoded paths that make the library only work for Google Colab's notebooks.

MagedSaeed · 2022-05-14T20:06:03Z

@sudomaze do not worry about the hardcoded, paths. I will try to figure it out somehow. At least, please make sure you pulled from the latest changes to fix the --gradient_checkpointing flag issue.

ma7dev · 2022-05-15T16:14:57Z

@MagedSaeed Per this commit, ca6935a, it seems that this PR was pulled from the latest changes, however, that commit didn't update run_classifier.py which aligns with the issue that was pointed out before. I will update run_classifier.py to have the same changes as run_common_voice.py and the other files.

MagedSaeed · 2022-05-16T15:28:30Z

@sudomaze I check from my side, and things seem to be fine for me. Thanks for the effort @sudomaze, Big Kudos. I am accepting the PR and applying the merge.

@zaidalyafeai I am getting errors when running the training scripts as I do not have the datasets on my machine. Can you please make sure things are running as expected with the datasets you have? Also, we need to make sure that we are managing paths properly.

ma7dev · 2022-05-16T15:47:33Z

@MagedSaeed thanks for the great feedback!

I will update #18 to include the leftovers from this PR like fixing hard-coded paths, the removal of FastSpeech2, utilizing huggingface, etc.

ma7dev added 2 commits April 29, 2022 07:09

removed requirements.txt and publish ci

fc7deae

zaidalyafeai assigned MagedSaeed Apr 29, 2022

MagedSaeed reviewed Apr 30, 2022

View reviewed changes

ckpts/.gitignore Outdated Show resolved Hide resolved

MagedSaeed reviewed Apr 30, 2022

View reviewed changes

Testing publish

a4b2fb9

ma7dev requested a review from MagedSaeed April 30, 2022 23:22

Added CI for publishing to PyPI

9c4b207

ma7dev added 2 commits May 1, 2022 04:18

remove test_publish ci

27c68d1

Merge pull request #1 from sudomaze/feat/publish_ci

e8c9c76

ma7dev added 2 commits May 15, 2022 09:16

remove grad_checkpointing per ARBML@ca6935a

a0f071b

Merge pull request #2 from sudomaze/feat/sudomaze-remove-checkpoint

7c6132b

MagedSaeed approved these changes May 16, 2022

View reviewed changes

MagedSaeed merged commit 290027b into ARBML:main May 16, 2022

ma7dev mentioned this pull request May 16, 2022

[Proposal] Codebase refactoring #18

Open

Refactored code and small changes #21

Refactored code and small changes #21

Conversation

ma7dev commented Apr 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ma7dev Apr 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MagedSaeed Apr 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ma7dev Apr 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ma7dev Apr 30, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MagedSaeed commented Apr 30, 2022

ma7dev commented Apr 30, 2022 • edited Loading

ma7dev commented Apr 30, 2022 • edited Loading

The latest commit's message:

ma7dev commented May 1, 2022 • edited Loading

MagedSaeed commented May 5, 2022

ma7dev commented May 5, 2022

MagedSaeed commented May 11, 2022 • edited Loading

ma7dev commented May 11, 2022 • edited Loading

zaidalyafeai commented May 11, 2022

ma7dev commented May 11, 2022 • edited Loading

zaidalyafeai commented May 11, 2022

ma7dev commented May 11, 2022

ma7dev commented May 11, 2022 • edited Loading

MagedSaeed commented May 11, 2022

ma7dev commented May 11, 2022 • edited Loading

zaidalyafeai commented May 11, 2022

MagedSaeed commented May 14, 2022

ma7dev commented May 14, 2022

MagedSaeed commented May 14, 2022

ma7dev commented May 15, 2022

MagedSaeed commented May 16, 2022

ma7dev commented May 16, 2022

ma7dev commented Apr 29, 2022 •

edited

Loading

ma7dev Apr 30, 2022 •

edited

Loading

MagedSaeed Apr 30, 2022 •

edited

Loading

ma7dev Apr 30, 2022 •

edited

Loading

ma7dev Apr 30, 2022 •

edited

Loading

ma7dev commented Apr 30, 2022 •

edited

Loading

ma7dev commented Apr 30, 2022 •

edited

Loading

ma7dev commented May 1, 2022 •

edited

Loading

MagedSaeed commented May 11, 2022 •

edited

Loading

ma7dev commented May 11, 2022 •

edited

Loading

ma7dev commented May 11, 2022 •

edited

Loading

ma7dev commented May 11, 2022 •

edited

Loading

ma7dev commented May 11, 2022 •

edited

Loading