keras icon indicating copy to clipboard operation
keras copied to clipboard

Published 20 hours ago • verified publisher icon keras-team

Updated timeseries_dataset_from_array

Open markub3327 opened this issue 3 years ago • 10 comments

Hello,

@divyashreepathihalli @qlzh727 @gowthamkpr solution of #16315

I added a feature for seq-to-seq, too. If targets are defined for each step in window, this is a useful feature.

Thanks.

markub3327 avatar May 13 '22 09:05 markub3327

I created a unit test for the element_shape as is mentioned in the issue.

markub3327 avatar May 13 '22 15:05 markub3327

I delete the seq_to_seq flag.

markub3327 avatar May 13 '22 16:05 markub3327

@qlzh727

I added the drop_remainder flag.

Thanks.

markub3327 avatar May 13 '22 17:05 markub3327

@qlzh727

I changed the seq_to_seq terminology to many_to_many as is described in the example 3 in docstring.

I point to Option 1 and Option 2 in the example as the thing with the same result, but cleaner code.

Thanks.

markub3327 avatar May 13 '22 18:05 markub3327

@fchollet

Please, Can you review it?

markub3327 avatar May 15 '22 13:05 markub3327

@markub3327 Can you please resolve conflicts? Thank you!

gbaned avatar Jun 10 '22 16:06 gbaned

@gbaned

Please concretize conflicts. It's not so clear to me. Thanks.

markub3327 avatar Jun 11 '22 17:06 markub3327

Hi @markub3327 Can you please resolve conflicts here https://github.com/keras-team/keras/pull/16533/conflicts ? Thank you!

gbaned avatar Aug 05 '22 07:08 gbaned

Hi @gbaned all is done. You can merge.

markub3327 avatar Aug 05 '22 09:08 markub3327

Now many_to_many functionality is provided by the old procedure .... original Example 3.

markub3327 avatar Aug 08 '22 21:08 markub3327

Hi @fchollet / @qlzh727 Any update on this PR? Please. Thank you!

gbaned avatar Dec 16 '22 13:12 gbaned

@fchollet @qlzh727

Here is the benchmark: https://gist.github.com/markub3327/7534c726208811e2312445455c7884c0

markub3327 avatar Mar 08 '23 20:03 markub3327

Took a look at the benchmark. Is looks like timeseries_dataset_from_array2, which I guessing is the new version, is actually performing noticeably worse. Is that correct? We will probably not be able to land this with a performance regression.

mattdangerw avatar Mar 17 '23 19:03 mattdangerw

Hi @markub3327 Any update on this PR? Please. Thank you!

gbaned avatar Apr 13 '23 10:04 gbaned

No..... my solution is using clearer code and more methods from TF.Dataset API, but it was slow.

markub3327 avatar Apr 13 '23 18:04 markub3327

Hello, Thank you for submitting a pull request.

We're currently in the process of migrating the new Keras 3 code base from keras-team/keras-core to keras-team/keras. Consequently, merging this PR is not possible at the moment. After the migration is successfully completed, feel free to reopen this PR at keras-team/keras if you believe it remains relevant to the Keras 3 code base. If instead this PR fixes a bug or security issue in legacy tf.keras, you can instead reopen the PR at keras-team/tf-keras, which hosts the TensorFlow-only, legacy version of Keras.

sachinprasadhs avatar Sep 19 '23 16:09 sachinprasadhs