-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
add AudioDiffusionPipeline and LatentAudioDiffusionPipeline #1334
Conversation
@patrickvonplaten @Vaibhavs10 I'd be very grateful if you could have a look at this. I'll fix the failing tests tomorrow. |
The documentation is not available anymore as the PR was closed or merged. |
src/diffusers/__init__.py
Outdated
LDMPipeline, | ||
LDMSuperResolutionPipeline, | ||
Mel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we remove this from the general __init__.py
function? -> I don't think one would use "Mel"
witouth the pipelines no? :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A mel instance is a parameter to the pipeline and it is useful for creating the dataset and training the model. I agree that it will probably only be used in conjunction with the pipelines, so as long as you can import it from diffusers.pipelines that should be Ok. Is that what you mean? Thanks!
[`DanceDiffusionPipeline`], [`AudioDiffusionPipeline`] and [`LatentAudioDiffusionPipeline`] can be used to generate |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool!
specific language governing permissions and limitations under the License. | ||
--> | ||
|
||
# Audio Diffusion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool docs!
@@ -36,6 +37,7 @@ RUN python3 -m pip install --no-cache-dir --upgrade pip && \ | |||
numpy \ | |||
scipy \ | |||
tensorboard \ | |||
transformers | |||
transformers \ | |||
librosa |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok for me @anton-l what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for reviewing @patrickvonplaten !
def test_audio_diffusion(self): | ||
device = torch_device | ||
|
||
mel = Mel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we maybe create this automatically directly in the pipeline? It might be a bit more user-friendly?
import numpy as np | ||
import torch | ||
|
||
from diffusers import ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice tests!
@torch.no_grad() | ||
def __call__( | ||
self, | ||
mel: Mel, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit weird to me that one has to pass a class here for __call__
-> wouldn't it be better to just do this inside the pipeline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @teticio,
This looks generally very nice to me and I think we can merge this soon :-)
One big thing we need to change is to make librosa
an optional dependency.
You can do this as follows:
- Add some "is_librosa_available()" logic here:
diffusers/src/diffusers/utils/import_utils.py
Line 129 in ab1f01e
_unidecode_available = importlib.util.find_spec("unidecode") is not None
And then import the pipeline only if librosa
is available. Maybe similar to how we do it for the LMSScheduler here:
diffusers/src/diffusers/__init__.py
Line 61 in ab1f01e
if is_torch_available() and is_scipy_available(): |
Also I would maybe not accept an "empyt" Mel()
class as an input to the call function, IMO that's a bit unintuitive design-wise - could we maybe just better create this inside the call method? Wdyt?
Finally, let's maybe not add Mel
to the public init as I don't think anybody would import just the Mel
class no?
Overall, great work though! Very happy to soon have a second audio diffusion model 😍
@@ -187,7 +188,8 @@ def run(self): | |||
"sentencepiece", | |||
"scipy", | |||
"torchvision", | |||
"transformers" | |||
"transformers", | |||
"librosa" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fine for me @anton-l wdyt?
@anton-l could you quickly check the docker file changes here regarding the new |
Great, thanks for the tip. I'll do that tomorrow.
The Mel class encapsulates a few parameters (like hop length and so on) which I decided against adding to the model configs, so as not to pollute things. However, I can default the parameter so that it creates a Mel with the default parameters. I initially decided against this because I wanted to make it explicit that these parameters need setting. Do you think the best thing is to add the parameters like hop length etc to the pipeline call with suitable defaults that are then used to create the mel object? Happy to follow your guidance here.
So I think the Mel class will be imported separately from the pipeline (for dataset creation and training). Shall I just make it importable from diffusers.pipelines and not from diffusers?
Thank you! Very excited to add my 2 cents to this excellent repo! |
...or maybe it makes more sense to pass the Mel object (or the parameters needed to instantiate it) as kwargs in the constructor instead of the call method. Let me know what you think. |
Hey @teticio, Sorry I think the commit history got messed up :-/ Yes, definitely fine for me to pass |
The documentation is not available anymore as the PR was closed or merged. |
author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041721 +0000 parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041704 +0000 add colab notebook [Flax] Fix loading scheduler from subfolder (#1319) [FLAX] Fix loading scheduler from subfolder Fix/Enable all schedulers for in-painting (#1331) * inpaint fix k lms * onnox as well * up Correct path to schedlure (#1322) * [Examples] Correct path * uP Avoid nested fix-copies (#1332) * Avoid nested `# Copied from` statements during `make fix-copies` * style Fix img2img speed with LMS-Discrete Scheduler (#896) Casting `self.sigmas` into a different dtype (the one of original_samples) is not advisable. In my img2img pipeline this leads to a long running time in the `integrate.quad` call later on- by long I mean more than 10x slower. Co-authored-by: Anton Lozhkov <[email protected]> Fix the order of casts for onnx inpainting (#1338) Legacy Inpainting Pipeline for Onnx Models (#1237) * Add legacy inpainting pipeline compatibility for onnx * remove commented out line * Add onnx legacy inpainting test * Fix slow decorators * pep8 styling * isort styling * dummy object * ordering consistency * style * docstring styles * Refactor common prompt encoding pattern * Update tests to permanent repository home * support all available schedulers until ONNX IO binding is available Co-authored-by: Anton Lozhkov <[email protected]> * updated styling from PR suggested feedback Co-authored-by: Anton Lozhkov <[email protected]> Jax infer support negative prompt (#1337) * support negative prompts in sd jax pipeline * pass batched neg_prompt * only encode when negative prompt is None Co-authored-by: Juan Acevedo <[email protected]> Update README.md: Minor change to Imagic code snippet, missing dir error (#1347) Minor change to Imagic Readme Missing dir causes an error when running the example code. make style change the sample model (#1352) * Update alt_diffusion.mdx * Update alt_diffusion.mdx Add bit diffusion [WIP] (#971) * Create bit_diffusion.py Bit diffusion based on the paper, arXiv:2208.04202, Chen2022AnalogBG * adding bit diffusion to new branch ran tests * tests * tests * tests * tests * removed test folders + added to README * Update README.md Co-authored-by: Patrick von Platen <[email protected]>
Yeah, sorry about that - not sure what I did wrong there, but it should be OK now.
So I took a different tack. I thought it would make more sense for Mel to be set up in the constructor of the pipeline. For that to be possible, it needs to be a module. So I moved it to models and derived it from ConfigMixin. For it to be able to be load_from_pretrained, I added Mel to the LOADABLE_CLASSES. The thinking is that this module could be replaced by some other audio<->image transformation in a composable way. For example, a neural one instead of the mel spectrogram. Arguably, there should be a base class (e.g., Audio2Image or something) from which Mel is derived, but I thought it might make more sense to refactor that if and when an alternative is implemented. The great advantage of doing it this way is that the Mel object is guaranteed to be consistent with the rest of the model; before the user had to make sure that he / she was using the configuration used to train the model in the first place. For this to work, I have to update the models uploaded to HF hub to include the Mel config. So currently the 'slow' test will fail. However, the colab notebook linked in the documentation is currently pointing to a modified version of the models in HF hub, so you can try it out with a pre-trained model there. [A consequence of this is that Mel is still importable from diffusers...] |
Done! |
@patrickvonplaten As soon as you confirm that this approach is acceptable (making Mel a module in the pipeline), I will make the corresponding changes in my current repo (from which I am migrating) and update the model repos accordingly, so that there can be a seamless switch over when the new version of diffusers comes out. Here is an example of how it will look in the model config: https://huggingface.co/teticio/latent-audio-diffusion-ddim-256-new/blob/main/mel/mel_config.json |
Ah, one last thing. If you don't like Mel being a LOADABLE_CLASS, maybe we could make ConfigMixin one instead. This would allow config of classes without model weights other than Schedulers to be loaded and saved. |
@patrickvonplaten I put you back as reviewer to check the changes I made to Mel ^. I didn't realize it would remove anton-I |
Sorry. I dk wtf happened with my repo. I am going to start a new PR from scratch |
) * add AudioDiffusionPipeline and LatentAudioDiffusionPipeline * add docs to toc * fix tests * fix tests * fix tests * fix tests * fix tests * Update pr_tests.yml Fix tests * parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041721 +0000 parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041704 +0000 add colab notebook [Flax] Fix loading scheduler from subfolder (#1319) [FLAX] Fix loading scheduler from subfolder Fix/Enable all schedulers for in-painting (#1331) * inpaint fix k lms * onnox as well * up Correct path to schedlure (#1322) * [Examples] Correct path * uP Avoid nested fix-copies (#1332) * Avoid nested `# Copied from` statements during `make fix-copies` * style Fix img2img speed with LMS-Discrete Scheduler (#896) Casting `self.sigmas` into a different dtype (the one of original_samples) is not advisable. In my img2img pipeline this leads to a long running time in the `integrate.quad` call later on- by long I mean more than 10x slower. Co-authored-by: Anton Lozhkov <[email protected]> Fix the order of casts for onnx inpainting (#1338) Legacy Inpainting Pipeline for Onnx Models (#1237) * Add legacy inpainting pipeline compatibility for onnx * remove commented out line * Add onnx legacy inpainting test * Fix slow decorators * pep8 styling * isort styling * dummy object * ordering consistency * style * docstring styles * Refactor common prompt encoding pattern * Update tests to permanent repository home * support all available schedulers until ONNX IO binding is available Co-authored-by: Anton Lozhkov <[email protected]> * updated styling from PR suggested feedback Co-authored-by: Anton Lozhkov <[email protected]> Jax infer support negative prompt (#1337) * support negative prompts in sd jax pipeline * pass batched neg_prompt * only encode when negative prompt is None Co-authored-by: Juan Acevedo <[email protected]> Update README.md: Minor change to Imagic code snippet, missing dir error (#1347) Minor change to Imagic Readme Missing dir causes an error when running the example code. make style change the sample model (#1352) * Update alt_diffusion.mdx * Update alt_diffusion.mdx Add bit diffusion [WIP] (#971) * Create bit_diffusion.py Bit diffusion based on the paper, arXiv:2208.04202, Chen2022AnalogBG * adding bit diffusion to new branch ran tests * tests * tests * tests * tests * removed test folders + added to README * Update README.md Co-authored-by: Patrick von Platen <[email protected]> * move Mel to module in pipeline construction, make librosa optional * fix imports * fix copy & paste error in comment * fix style * add missing register_to_config * fix class docstrings * fix class docstrings * tweak docstrings * tweak docstrings * update slow test * put trailing commas back * respect alphabetical order * remove LatentAudioDiffusion, make vqvae optional * move Mel from models back to pipelines :-) * allow loading of pretrained audiodiffusion models * fix tests * fix dummies * remove reference to latent_audio_diffusion in docs * unused import * inherit from SchedulerMixin to make loadable * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Patrick von Platen <[email protected]>
…ce#1334 (huggingface#1426) * add AudioDiffusionPipeline and LatentAudioDiffusionPipeline * add docs to toc * fix tests * fix tests * fix tests * fix tests * fix tests * Update pr_tests.yml Fix tests * parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041721 +0000 parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041704 +0000 add colab notebook [Flax] Fix loading scheduler from subfolder (huggingface#1319) [FLAX] Fix loading scheduler from subfolder Fix/Enable all schedulers for in-painting (huggingface#1331) * inpaint fix k lms * onnox as well * up Correct path to schedlure (huggingface#1322) * [Examples] Correct path * uP Avoid nested fix-copies (huggingface#1332) * Avoid nested `# Copied from` statements during `make fix-copies` * style Fix img2img speed with LMS-Discrete Scheduler (huggingface#896) Casting `self.sigmas` into a different dtype (the one of original_samples) is not advisable. In my img2img pipeline this leads to a long running time in the `integrate.quad` call later on- by long I mean more than 10x slower. Co-authored-by: Anton Lozhkov <[email protected]> Fix the order of casts for onnx inpainting (huggingface#1338) Legacy Inpainting Pipeline for Onnx Models (huggingface#1237) * Add legacy inpainting pipeline compatibility for onnx * remove commented out line * Add onnx legacy inpainting test * Fix slow decorators * pep8 styling * isort styling * dummy object * ordering consistency * style * docstring styles * Refactor common prompt encoding pattern * Update tests to permanent repository home * support all available schedulers until ONNX IO binding is available Co-authored-by: Anton Lozhkov <[email protected]> * updated styling from PR suggested feedback Co-authored-by: Anton Lozhkov <[email protected]> Jax infer support negative prompt (huggingface#1337) * support negative prompts in sd jax pipeline * pass batched neg_prompt * only encode when negative prompt is None Co-authored-by: Juan Acevedo <[email protected]> Update README.md: Minor change to Imagic code snippet, missing dir error (huggingface#1347) Minor change to Imagic Readme Missing dir causes an error when running the example code. make style change the sample model (huggingface#1352) * Update alt_diffusion.mdx * Update alt_diffusion.mdx Add bit diffusion [WIP] (huggingface#971) * Create bit_diffusion.py Bit diffusion based on the paper, arXiv:2208.04202, Chen2022AnalogBG * adding bit diffusion to new branch ran tests * tests * tests * tests * tests * removed test folders + added to README * Update README.md Co-authored-by: Patrick von Platen <[email protected]> * move Mel to module in pipeline construction, make librosa optional * fix imports * fix copy & paste error in comment * fix style * add missing register_to_config * fix class docstrings * fix class docstrings * tweak docstrings * tweak docstrings * update slow test * put trailing commas back * respect alphabetical order * remove LatentAudioDiffusion, make vqvae optional * move Mel from models back to pipelines :-) * allow loading of pretrained audiodiffusion models * fix tests * fix dummies * remove reference to latent_audio_diffusion in docs * unused import * inherit from SchedulerMixin to make loadable * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Patrick von Platen <[email protected]>
…ce#1334 (huggingface#1426) * add AudioDiffusionPipeline and LatentAudioDiffusionPipeline * add docs to toc * fix tests * fix tests * fix tests * fix tests * fix tests * Update pr_tests.yml Fix tests * parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041721 +0000 parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041704 +0000 add colab notebook [Flax] Fix loading scheduler from subfolder (huggingface#1319) [FLAX] Fix loading scheduler from subfolder Fix/Enable all schedulers for in-painting (huggingface#1331) * inpaint fix k lms * onnox as well * up Correct path to schedlure (huggingface#1322) * [Examples] Correct path * uP Avoid nested fix-copies (huggingface#1332) * Avoid nested `# Copied from` statements during `make fix-copies` * style Fix img2img speed with LMS-Discrete Scheduler (huggingface#896) Casting `self.sigmas` into a different dtype (the one of original_samples) is not advisable. In my img2img pipeline this leads to a long running time in the `integrate.quad` call later on- by long I mean more than 10x slower. Co-authored-by: Anton Lozhkov <[email protected]> Fix the order of casts for onnx inpainting (huggingface#1338) Legacy Inpainting Pipeline for Onnx Models (huggingface#1237) * Add legacy inpainting pipeline compatibility for onnx * remove commented out line * Add onnx legacy inpainting test * Fix slow decorators * pep8 styling * isort styling * dummy object * ordering consistency * style * docstring styles * Refactor common prompt encoding pattern * Update tests to permanent repository home * support all available schedulers until ONNX IO binding is available Co-authored-by: Anton Lozhkov <[email protected]> * updated styling from PR suggested feedback Co-authored-by: Anton Lozhkov <[email protected]> Jax infer support negative prompt (huggingface#1337) * support negative prompts in sd jax pipeline * pass batched neg_prompt * only encode when negative prompt is None Co-authored-by: Juan Acevedo <[email protected]> Update README.md: Minor change to Imagic code snippet, missing dir error (huggingface#1347) Minor change to Imagic Readme Missing dir causes an error when running the example code. make style change the sample model (huggingface#1352) * Update alt_diffusion.mdx * Update alt_diffusion.mdx Add bit diffusion [WIP] (huggingface#971) * Create bit_diffusion.py Bit diffusion based on the paper, arXiv:2208.04202, Chen2022AnalogBG * adding bit diffusion to new branch ran tests * tests * tests * tests * tests * removed test folders + added to README * Update README.md Co-authored-by: Patrick von Platen <[email protected]> * move Mel to module in pipeline construction, make librosa optional * fix imports * fix copy & paste error in comment * fix style * add missing register_to_config * fix class docstrings * fix class docstrings * tweak docstrings * tweak docstrings * update slow test * put trailing commas back * respect alphabetical order * remove LatentAudioDiffusion, make vqvae optional * move Mel from models back to pipelines :-) * allow loading of pretrained audiodiffusion models * fix tests * fix dummies * remove reference to latent_audio_diffusion in docs * unused import * inherit from SchedulerMixin to make loadable * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Patrick von Platen <[email protected]>
…ce#1334 (huggingface#1426) * add AudioDiffusionPipeline and LatentAudioDiffusionPipeline * add docs to toc * fix tests * fix tests * fix tests * fix tests * fix tests * Update pr_tests.yml Fix tests * parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041721 +0000 parent 499ff34 author teticio <[email protected]> 1668765652 +0000 committer teticio <[email protected]> 1669041704 +0000 add colab notebook [Flax] Fix loading scheduler from subfolder (huggingface#1319) [FLAX] Fix loading scheduler from subfolder Fix/Enable all schedulers for in-painting (huggingface#1331) * inpaint fix k lms * onnox as well * up Correct path to schedlure (huggingface#1322) * [Examples] Correct path * uP Avoid nested fix-copies (huggingface#1332) * Avoid nested `# Copied from` statements during `make fix-copies` * style Fix img2img speed with LMS-Discrete Scheduler (huggingface#896) Casting `self.sigmas` into a different dtype (the one of original_samples) is not advisable. In my img2img pipeline this leads to a long running time in the `integrate.quad` call later on- by long I mean more than 10x slower. Co-authored-by: Anton Lozhkov <[email protected]> Fix the order of casts for onnx inpainting (huggingface#1338) Legacy Inpainting Pipeline for Onnx Models (huggingface#1237) * Add legacy inpainting pipeline compatibility for onnx * remove commented out line * Add onnx legacy inpainting test * Fix slow decorators * pep8 styling * isort styling * dummy object * ordering consistency * style * docstring styles * Refactor common prompt encoding pattern * Update tests to permanent repository home * support all available schedulers until ONNX IO binding is available Co-authored-by: Anton Lozhkov <[email protected]> * updated styling from PR suggested feedback Co-authored-by: Anton Lozhkov <[email protected]> Jax infer support negative prompt (huggingface#1337) * support negative prompts in sd jax pipeline * pass batched neg_prompt * only encode when negative prompt is None Co-authored-by: Juan Acevedo <[email protected]> Update README.md: Minor change to Imagic code snippet, missing dir error (huggingface#1347) Minor change to Imagic Readme Missing dir causes an error when running the example code. make style change the sample model (huggingface#1352) * Update alt_diffusion.mdx * Update alt_diffusion.mdx Add bit diffusion [WIP] (huggingface#971) * Create bit_diffusion.py Bit diffusion based on the paper, arXiv:2208.04202, Chen2022AnalogBG * adding bit diffusion to new branch ran tests * tests * tests * tests * tests * removed test folders + added to README * Update README.md Co-authored-by: Patrick von Platen <[email protected]> * move Mel to module in pipeline construction, make librosa optional * fix imports * fix copy & paste error in comment * fix style * add missing register_to_config * fix class docstrings * fix class docstrings * tweak docstrings * tweak docstrings * update slow test * put trailing commas back * respect alphabetical order * remove LatentAudioDiffusion, make vqvae optional * move Mel from models back to pipelines :-) * allow loading of pretrained audiodiffusion models * fix tests * fix dummies * remove reference to latent_audio_diffusion in docs * unused import * inherit from SchedulerMixin to make loadable * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Patrick von Platen <[email protected]>
I have added
AudioDiffusionPipeline
andLatentAudioDiffusionPipeline
which I intend to migrate fromhttps://github.com/teticio/audio-diffusion
. I have added them to the mainsrc
as opposed to the community pipelines due to the inheritance ofLatentAudioDiffusionPipeline
fromAudioDiffusionPipeline
, which cannot be done in a single pipeline file, as well as the fact that theMel
class is needed to convert from audio to images and vice versa. It might make sense to move theMel
class somewhere more central, as it could be used by other pipelines.