keras icon indicating copy to clipboard operation
keras copied to clipboard

Add `MelSpectrogram` layer

Open awsaf49 opened this issue 2 years ago • 9 comments

I created a PR for MelSpectrogram layer on keras-team/keras repo https://github.com/keras-team/keras/pull/17717. But as keras-core will become Keras 3.0 and subsume tf.keras I think this layer should be in keras-core with backend agnostic feature.

I can re-implement that in keras-core with keras.ops.stft but I may have to implement one additional ops linear_to_mel_weight_matrix.

awsaf49 avatar Sep 15 '23 13:09 awsaf49

cc: @fchollet

awsaf49 avatar Sep 15 '23 13:09 awsaf49

Hi, thanks for the suggestion. Yes, I think this can be a useful layer. Where you would we put it? in layers/preprocessing?

fchollet avatar Sep 21 '23 17:09 fchollet

Yes. layers/preprocessing would be nice. I will create the PR soon with previous progress.

awsaf49 avatar Sep 21 '23 17:09 awsaf49

@fchollet , I tried to implement the linear_to_mel_weight_matrix in keras_core for MelSpectrogram layer, and during this, I came across a few other dependencies that seem to be missing in keras_core. Specifically, functions like tf.signal.frame -> tf.gather, tf.strided_slice are yet to be added.

Additionally, I observed that TensorFlow uses tensor_util.constant_value() to determine static values, but I'm not sure how that translates in KerasCore. For reference, you can see its usage here.

I can add this layer to tf-keras (keras-team/tf-keras/issues/55) with few adjustments, especially considering the progress made before the migration. However, adding this layer to keras_core will require these missing dependencies to be added. I did attempt to implement them, but some of the code seems to be deeply rooted in array_ops.cc.

awsaf49 avatar Sep 25 '23 11:09 awsaf49

@awsaf49 , Do you think this PR is still valid for Keras 3, if you think so please revisit this PR https://github.com/keras-team/keras/pull/17717 and open it. Else, feel free to close this issue. Thanks!

sachinprasadhs avatar Dec 11 '23 04:12 sachinprasadhs

Some features are available on TF but not in Keras 3.0 yet. As soon as they are available I can complete the PR. Should I consider closing it?

awsaf49 avatar Dec 11 '23 04:12 awsaf49

i can contribute for things like adding gather and some other ops (eg. tf.image.*) to unblock you since i need these ops for my use cases anyways.

generally these ops can be generally implemented with a combination of existing keras ops, so i'd like to know

  1. where should i put these ops? i am guessing keras.ops.nn if it is ops like gather ?
  2. when i am implementing, should i still do backend.nn.xx or it is preferred to call existing function in keras.ops.nn?

haohuanw avatar Dec 29 '23 22:12 haohuanw

@haohuanw thanks, that would be great. Btw, tf.gather is already available at Keras with ops.take. I will only be needing tf.signal.frame, then I will be good to go.

cc: @fchollet

awsaf49 avatar Dec 30 '23 03:12 awsaf49

ah ok, i need the version of gather with batch_dims > 0 that's why normal take doesn't work for me; but if you don't need that take should be enough

haohuanw avatar Dec 30 '23 17:12 haohuanw