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

Example code for Spectrogram in documentation #1566

Merged
merged 10 commits into from
Aug 4, 2021
Merged

Example code for Spectrogram in documentation #1566

merged 10 commits into from
Aug 4, 2021

Conversation

harishsdev
Copy link
Contributor

@harishsdev
Copy link
Contributor Author

PR for : #1564

Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Thanks for the con tribulation but your base branch is old more than one month. Please rebase it on master.

@@ -60,6 +60,20 @@ class Spectrogram(torch.nn.Module):
dimension for real and imaginary parts. (see ``torch.view_as_real``).
When ``power`` is provided, the value must be False, as the resulting
Tensor represents real-valued power.

Example:-
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. Indentation is off.
  2. hyphen and colon are unnecessary.

Please follow an existing example;

Example
>>> specgram = torch.randn(1, 40, 1000)
>>> delta = compute_deltas(specgram)
>>> delta2 = compute_deltas(delta)

Sample rate of waveform: 8000
>>> specgram = torchaudio.transforms.Spectrogram()(waveform)
>>> print("Shape of spectrogram: {}".format(specgram.size()))
Shape of spectrogram: torch.Size([2, 201, 1342])
Copy link
Collaborator

Choose a reason for hiding this comment

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

This example spends 6 lines for loading audio tensor and 3 lines for the Transform. Instead please make an example around the transform.

Make the resulting transform instance an intermediate named variable.
Also please use some constructor arguments for showcasing it.

@harishsdev
Copy link
Contributor Author

@mthrok Thanks for inputs

I have forked the repository and created Spectrogram branch with following commands rebased,please give me some inputs if i am wrong

git remote add upstream https://github.com/pytorch/audio.git
git fetch upstream

git checkout master

git rebase upstream/master

git push -f origin master

@harishsdev harishsdev requested a review from mthrok June 12, 2021 03:10
@harishsdev
Copy link
Contributor Author

How to run flake in torchaudio for checking pep8 style coding, please suggest

Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

Hi @harishsdev

You can simple do flake8 torchaudio for flake8.

>>> specgram = torch.randn(1, 40, 1000)
>>> specgram = torchaudio.transforms.Spectrogram()(specgram)
>>> specgram.shape
torch.Size([1, 40, 201, 6])
Copy link
Collaborator

Choose a reason for hiding this comment

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

  1. This indentation is too much, there should be only 4 spaces instead of 8.
  2. Please assign the transform into intermediate variable.
  3. Please provide some arguments to Spectrogram constructor.
  4. The input to Spectrogram should be called waveform.

@harishsdev harishsdev requested a review from mthrok June 21, 2021 10:07
@mthrok
Copy link
Collaborator

mthrok commented Jul 2, 2021

@harishsdev

Please build the doc by yourself or check the artifact of build_docs job.
The indentation is off.

https://239906-90321822-gh.circle-artifacts.com/0/docs/transforms.html#spectrogram

Screen Shot 2021-07-02 at 14 21 37

Also please, do assign the transform object to an intermediate variable to showcase the instantiation of Transform and application of the transform separately. Please do not write them in one line.

Copy link
Collaborator

@mthrok mthrok left a comment

Choose a reason for hiding this comment

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

  1. Please fix the indentation. Build the doc and see that the rendering is correct.
  2. Please assign the transform into intermediate variable.

Example

>>> waveform, sample_rate = torchaudio.load('test.wav', normalization=True)
>>> transformed_spectrogram = torchaudio.transforms.Spectrogram(n_fft = 800)(waveform)
Copy link
Member

Choose a reason for hiding this comment

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

Hi @harishsdev. Could you assign the transform parameter and move the forward method to a separate line? You can see #1644 as an example.

transform = torchaudio.transforms.Spectrogram(n_fft = 512)
spectrogram = transform(waveform)

@harishsdev
Copy link
Contributor Author

harishsdev commented Aug 3, 2021 via email

@harishsdev harishsdev requested a review from nateanl August 4, 2021 04:36
Copy link
Member

@nateanl nateanl left a comment

Choose a reason for hiding this comment

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

Same as #1586. We can merge it after addressing the nits.


Example

>>> waveform, sample_rate = torchaudio.load('test.wav', normalization=True)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> waveform, sample_rate = torchaudio.load('test.wav', normalization=True)
>>> waveform, sample_rate = torchaudio.load('test.wav', normalize=True)

Example

>>> waveform, sample_rate = torchaudio.load('test.wav', normalization=True)
>>> transform = torchaudio.transforms.Spectrogram(n_fft = 800)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
>>> transform = torchaudio.transforms.Spectrogram(n_fft = 800)
>>> transform = torchaudio.transforms.Spectrogram(n_fft=800)

@@ -67,6 +67,13 @@ class Spectrogram(torch.nn.Module):
This argument is only effective when ``power=None``. It is ignored for
cases where ``power`` is a number as in those cases, the returned tensor is
power spectrogram, which is a real-valued tensor.

Example

Copy link
Member

Choose a reason for hiding this comment

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

Same for this one. Let's delete the line break.

Suggested change

@harishsdev harishsdev requested a review from nateanl August 4, 2021 05:23
@nateanl nateanl merged commit 1d6b15e into pytorch:main Aug 4, 2021
mthrok added a commit to mthrok/audio that referenced this pull request Dec 13, 2022
* Update build.sh

Picks up 1.9 build from test.

* Update build.sh

* Update lite interpreter tutorial to beta (pytorch#1549)

* Update lite interpreter tutorial to beta

* Update lite interpreter to beta

* update model export script

* address comment and update documentation

* add custome build in first paragraph

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Update prototype_source/lite_interpreter.rst

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* replace file name

* update ios part

Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>

* Revert "Update lite interpreter tutorial to beta (pytorch#1549)" (pytorch#1569)

This reverts commit a702ca0fafe9d4a1ee0c1e4331de66245ceb3103.

* Update build.sh

* Update build.sh

* updated pipeline tutorial (pytorch#1562)

* reduce (pytorch#1546)

* Update seq2seq_translation_tutorial.py (pytorch#1532)

Co-authored-by: Holly Sweeney <77758406+holly1238@users.noreply.github.com>

* added CPU optimization guide part into tuning_guide (pytorch#1512)

* added CPU optimization guide part into tuning_guide

* changed non-python command to python comments in CPU specific optimization section

* Update tuning_guide.py

Changed comment of bash commands to double quote.

* Update tuning_guide.py

Co-authored-by: Brian Johnson <brianjo@fb.com>

* Typo fix (pytorch#1538)

Co-authored-by: Holly Sweeney <77758406+holly1238@users.noreply.github.com>

* Typo fix in text sentiment tutorial (pytorch#1543)

Trivial typo fix in docs

* Update dcgan_faces_tutorial.py (pytorch#1550)

Co-authored-by: Holly Sweeney <77758406+holly1238@users.noreply.github.com>

* updated pipeline tutorial

Co-authored-by: define_liuyi <793753866@qq.com>
Co-authored-by: dhayeah <57786651+dhayeah@users.noreply.github.com>
Co-authored-by: Holly Sweeney <77758406+holly1238@users.noreply.github.com>
Co-authored-by: Jing Xu <jing.xu@intel.com>
Co-authored-by: Brian Johnson <brianjo@fb.com>
Co-authored-by: Andrew C. Freeman <andrew.freeman@cawb.com>
Co-authored-by: Davide Fiocco <davidefiocco@users.noreply.github.com>
Co-authored-by: universuen <52519513+universuen@users.noreply.github.com>

* Update audio manipulation tutorial  (pytorch#1566)

* add resampling tutorial

* update benchmarking and sectioning

* remove np import

* Update torchaudio tutorial

* update resample dtype initialization

Co-authored-by: moto <855818+mthrok@users.noreply.github.com>

* updated text sentiment tutorial (pytorch#1563)

* updated transformer tutorial (pytorch#1565)

* Update numeric_suite_tutorial.py

s/Logger=/logger_cls=/

* Update profiler recipe doc (1.9) (pytorch#1528)

Summary:
Update the profiler recipe to use the new API and features

Test Plan:
make html-noplot

Co-authored-by: Brian Johnson <brianjo@fb.com>

* Update build.sh

Co-authored-by: cccclai <chenlai@fb.com>
Co-authored-by: Raziel <129535+raziel@users.noreply.github.com>
Co-authored-by: parmeet <parmeetbhatia@fb.com>
Co-authored-by: define_liuyi <793753866@qq.com>
Co-authored-by: dhayeah <57786651+dhayeah@users.noreply.github.com>
Co-authored-by: Holly Sweeney <77758406+holly1238@users.noreply.github.com>
Co-authored-by: Jing Xu <jing.xu@intel.com>
Co-authored-by: Andrew C. Freeman <andrew.freeman@cawb.com>
Co-authored-by: Davide Fiocco <davidefiocco@users.noreply.github.com>
Co-authored-by: universuen <52519513+universuen@users.noreply.github.com>
Co-authored-by: Caroline Chen <carolinechen@fb.com>
Co-authored-by: moto <855818+mthrok@users.noreply.github.com>
Co-authored-by: Nikita Shulga <nshulga@fb.com>
Co-authored-by: ilia-cher <30845429+ilia-cher@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants