Skip to content

Conversation

@ssnl
Copy link
Collaborator

@ssnl ssnl commented Dec 8, 2017

This PR

  1. Implements short-time fourier transform add torch.stft and torch.fft #3775 with tests against Scipy.
  2. Adds atan2 for cuda double & half tensors. Refactored atan2 to THCNumerics, Op to THCTensorMathPointwise.cuh.
  3. Adds support for python only default init in the parsers. This allows us to write argument with a default value that can either cause ambiguity in c++ (e.g., Scalar p in norm) or have a type that doesn't allow default value None/NULL/nullptr (e.g., int64_t fft_size in stft).
    a. Changes norm to also use this in both Declarations.cwrap for torch.norm named arguments do not work as expected #1419 . When Tensor and Variable merge, it should fix the issues in torch.norm named arguments do not work as expected #1419 .
  4. Implements three commonly used window functions (Hann, Hamming, Bartlett) with tests against Scipy

On 3: Now in python_variable_methods.cpp, if an argument has python only default value, it will be generated like the following. I added the comment for clarification purpose. It won't be in actual code.

static PyObject * THPVariable_stft(PyObject* self, PyObject* args, PyObject* kwargs)
{
  HANDLE_TH_ERRORS
  static PythonArgParser parser({
    "stft(int64_t frame_length, int64_t hop, int64_t fft_size=None, Tensor window=None, int64_t pad_end=0)",
  });
  auto& self_ = reinterpret_cast<THPVariable*>(self)->cdata;
  PyObject* parsed_args[6];
  auto r = parser.parse(args, kwargs, parsed_args);
  if (r.idx == 0) {
    Tensor & self = self_;
    int64_t frame_length = r.toInt64(0);
    int64_t hop = r.toInt64(1);
    const Tensor & window = r.tensor(3);
    int64_t pad_end = r.toInt64(4);
    // fft_size has python only default, so it will be generated last
    int64_t fft_size = r.toInt64WithDefault(2, frame_length);  
    return wrap(dispatch_stft(self, frame_length, hop, fft_size, window, pad_end));
  }
  Py_RETURN_NONE;
  END_HANDLE_TH_ERRORS
}

@pytorchbot
Copy link
Collaborator

@ssnl, thanks for your PR! We identified @zdevito to be a potential reviewer.

@ssnl ssnl closed this Dec 9, 2017
@ssnl ssnl reopened this Dec 9, 2017

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@ssnl ssnl changed the title Implement stft [WIP] Implement stft Dec 12, 2017
@ssnl ssnl force-pushed the stft branch 2 times, most recently from 416043f to 9af456f Compare December 13, 2017 00:01
@ssnl
Copy link
Collaborator Author

ssnl commented Dec 13, 2017

See above for what this PR contains.

@ssnl ssnl force-pushed the stft branch 3 times, most recently from 050e9a7 to ad46de8 Compare December 13, 2017 00:12

This comment was marked as off-topic.

@ssnl ssnl changed the title [WIP] Implement stft Add python only default init expression; Implement stft, hann/hamming/bartlett window. Dec 13, 2017
@ssnl ssnl closed this Dec 13, 2017
@ssnl ssnl reopened this Dec 13, 2017
@ssnl ssnl changed the title Add python only default init expression; Implement stft, hann/hamming/bartlett window. [WIP] Add python only default init expression; Implement stft, hann/hamming/bartlett window. Dec 13, 2017
@ssnl ssnl changed the title [WIP] Add python only default init expression; Implement stft, hann/hamming/bartlett window. Add python only default init expression; Implement stft, hann/hamming/bartlett window. Dec 14, 2017
@ezyang
Copy link
Contributor

ezyang commented Dec 14, 2017

@pytorchbot retest this please

1 similar comment
@ezyang
Copy link
Contributor

ezyang commented Dec 14, 2017

@pytorchbot retest this please

@ssnl ssnl force-pushed the stft branch 3 times, most recently from 03be651 to 2f42b55 Compare December 15, 2017 19:38
@soumith
Copy link
Contributor

soumith commented Dec 18, 2017

@ssnl once you resolve merge conflict, this is good to go.

@ssnl
Copy link
Collaborator Author

ssnl commented Dec 18, 2017

@soumith merged & tested :) thanks!

@soumith soumith merged commit d8b2e5d into pytorch:master Dec 18, 2017
@ssnl ssnl deleted the stft branch December 18, 2017 17:29
@ssnl ssnl restored the stft branch December 19, 2017 17:07
@ssnl ssnl deleted the stft branch December 19, 2017 17:09
@soumith soumith mentioned this pull request Jan 2, 2018
5 tasks
@faroit
Copy link

faroit commented Apr 25, 2018

@ssnl I think it would be nice to also have an inverse stft

@ssnl
Copy link
Collaborator Author

ssnl commented Apr 26, 2018

@faroit Yes, it's on my list of things to do. Thanks for bringing it up.

@faroit
Copy link

faroit commented Apr 26, 2018

@ssnl if you need help e.g. testing perfect reconstruction etc. just ping me

@ssnl
Copy link
Collaborator Author

ssnl commented Apr 26, 2018

@faroit thanks!

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.

7 participants