Skip to content
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

update(requirements) added missing fairseq #839

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Eidenz
Copy link
Contributor

@Eidenz Eidenz commented Sep 13, 2023

Fairseq was recently added on master branch but is missing in requirements, causing a crash when selecting an RVC model to load.

@Rafacasari
Copy link
Contributor

I noticed that too, I was about to ask why @InsanEagle removed fairseq. I think it was a typo or idk.

Also, @w-okada why don't remove fairseq completely? We can use transformers instead, idk if is faster but is an option

@InsanEagle
Copy link
Contributor

I noticed that too, I was about to ask why @InsanEagle removed fairseq. I think it was a typo or idk.

Also, @w-okada why don't remove fairseq completely? We can use transformers instead, idk if is faster but is an option

Idk why, but Google Colab can't install fairseq from requirements.txt. Also it was dublicated via !pip install so I removed it from requirements.txt. I tried to find fast solution for myself and didn't know that this file was being used somewhere else. I think it's fine to keep requirements.txt as it was or just update some libraries versions and keep !pip install pyworld fairseq in Google Colab as it works for now. Most likely there is a better solution, but I can’t offer it because I’m not very good in Python

@Eidenz
Copy link
Contributor Author

Eidenz commented Sep 13, 2023

The requirement file is used everywhere when building from source, it's also very common for Python projects.

So yes, this should be reverted. If Colab had issues with it, another workaround should be applied rather than modifying the main project requirements/server files (or making sure the modification doesn't alter the rest of the project).

@InsanEagle
Copy link
Contributor

InsanEagle commented Sep 13, 2023

The requirement file is used everywhere when building from source, it's also very common for Python projects.

So yes, this should be reverted. If Colab had issues with it, another workaround should be applied rather than modifying the main project requirements/server files (or making sure the modification doesn't alter the rest of the project).

I completely agree with you. it might be a good idea to create a separate requirements_colab.txt file or just install all dependencies using Google Colab script.

Also, if you need to return the requirements.txt to its original state, then in addition to fairseq, you also need to add pyworld. Version 0.3.3 as it was or 0.3.4 the newest one at your discretion.

@Rafacasari
Copy link
Contributor

Rafacasari commented Sep 14, 2023

Idk why, but Google Colab can't install fairseq from requirements.txt. Also it was dublicated via !pip install so I removed it from requirements.txt. I tried to find fast solution for myself and didn't know that this file was being used somewhere else. I think it's fine to keep requirements.txt as it was or just update some libraries versions and keep !pip install pyworld fairseq in Google Colab as it works for now. Most likely there is a better solution, but I can’t offer it because I’m not very good in Python

From my tests on Kaggle, fairseq don't build with the --no-build-isolation argument. That's why I was installing the fairseq before installing the requirements.txt on Colab.

Installing pyworld on a different line should work. I'll try to update the Colab files since now I have more knowledge about notebooks/python.

The Colab Notebook was kinda hardcoded to work but I'll re-make some cells to prevent more issues.

@GatienDoesStuff
Copy link

GatienDoesStuff commented Sep 14, 2023

Bump pyworld to 0.3.4, as 0.3.3 doesn't seem to be able to build on recent linux systems.

Never got 0.3.3 to compile locally, and what I'm assuming was a colab VM update also broke it recently. Bumping the minor revision fixes it and doesn't seem to have any drawbacks.

@Eidenz
Copy link
Contributor Author

Eidenz commented Sep 14, 2023

0.3.3 doesn't seem to be able to build on recent linux systems

That is very odd as we specifically locked the Dockerfile to pyworld 0.3.3 because 0.3.4 is breaking the building of wheels...
The Dockerfile is pulling an ubuntu22.04 image FYI.

Have you tested on linux installations other than your system?

edit: full instruction on Dockerfile:

pip install numpy==1.23.5 \
        && pip install pyworld==0.3.3 --no-build-isolation \
        && pip install -r requirements.txt

see #502

@GatienDoesStuff
Copy link

GatienDoesStuff commented Sep 14, 2023

I don't know what is up, but pyworld 0.3.3 doesn't build wheel on a fresh Ubuntu 22.04 container here.
pip install pyworld==0.3.3

        File "/usr/lib/python3.10/json/encoder.py", line 438, in _iterencode
          o = _default(o)
        File "/usr/lib/python3.10/json/encoder.py", line 179, in default
          raise TypeError(f'Object of type {o.__class__.__name__} '
      TypeError: Object of type PosixPath is not JSON serializable
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pyworld
Failed to build pyworld
ERROR: Could not build wheels for pyworld, which is required to install pyproject.toml-based projects

Same issue also happens on both of my Arch systems, as well as the google colab platform. Not sure what to think of it.
All systems are running their own python 3.10 venv.

Bumping to 0.3.4 allows it to build again

@Rafacasari
Copy link
Contributor

I don't know what is up, but pyworld 0.3.3 doesn't build wheel on a fresh Ubuntu 22.04 container here. pip install pyworld==0.3.3

        File "/usr/lib/python3.10/json/encoder.py", line 438, in _iterencode
          o = _default(o)
        File "/usr/lib/python3.10/json/encoder.py", line 179, in default
          raise TypeError(f'Object of type {o.__class__.__name__} '
      TypeError: Object of type PosixPath is not JSON serializable
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pyworld
Failed to build pyworld
ERROR: Could not build wheels for pyworld, which is required to install pyproject.toml-based projects

Same issue also happens on both of my Arch systems, as well as the google colab platform. Not sure what to think of it. All systems are running their own python 3.10 venv.

Bumping to 0.3.4 allows it to build again

Using --no-build-isolation argument don't fix it in pyworld==0.3.3?
pip install pyworld==0.3.3 --no-build-isolation

@GatienDoesStuff
Copy link

--no-build-isolation does indeed fix it, granted that the wheel package is installed (and also numpy but requirements.txt already pulls it)

I'm gonna do some more testing to see if 0.3.4 really can't build on some systems.

I still believe 0.3.4 is a better fit, as it just works with venvs, without having to fiddle with anything, compared to 0.3.3 which requires the no build isolation flag, as well as installing the wheel package beforehand.

I also took a look at the current dockerfile, and I don't really agree with it. While it may work in it's current state, mixing packages with apt & pip isn't a good idea (though one could argue it doesn't matter here as the packages are frozen).
Future docker images for cuda will eventually be on ubuntu 24.04, which will have https://peps.python.org/pep-0668/ implemented (ubuntu 23, fedora 38, debian 12, and arch already do).

For the sake of simplicity, if we can confirm that 0.3.4 is indeed more reliable, let's just go with that. Nothing is preventing the dockerfile from working around it, which it already is.

@s-b-repo
Copy link

still broken for me

I don't know what is up, but pyworld 0.3.3 doesn't build wheel on a fresh Ubuntu 22.04 container here. pip install pyworld==0.3.3

        File "/usr/lib/python3.10/json/encoder.py", line 438, in _iterencode
          o = _default(o)
        File "/usr/lib/python3.10/json/encoder.py", line 179, in default
          raise TypeError(f'Object of type {o.__class__.__name__} '
      TypeError: Object of type PosixPath is not JSON serializable
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pyworld
Failed to build pyworld
ERROR: Could not build wheels for pyworld, which is required to install pyproject.toml-based projects

Same issue also happens on both of my Arch systems, as well as the google colab platform. Not sure what to think of it. All systems are running their own python 3.10 venv.
Bumping to 0.3.4 allows it to build again

Using --no-build-isolation argument don't fix it in pyworld==0.3.3? pip install pyworld==0.3.3 --no-build-isolation

@s-b-repo
Copy link

s-b-repo commented Sep 20, 2023

  • Then installing a older version of cython with "pip install cython==0.29.36"

  • And then installing pyworld with "pip install pyworld==0.3.3 --no-build-isolation"

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.

5 participants