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

generalize batchseq to sequence of generic arrays #126

Merged
merged 3 commits into from
Oct 28, 2022
Merged

Conversation

CarloLucibello
Copy link
Member

@CarloLucibello CarloLucibello commented Oct 23, 2022

Implement the following behavior for batchseq: Given an input list xs containing B arrays of size * x Li, where * is common to all arrays and Li can vary, return a list of length Lmax where the elements have size * x B.

Have to depend on NNlib for this, not sure it is worth it.

Also, we deprecate rpad since it is piracy of a Base function in favor of a more generic rpad_constant (mimicking the NNlib naming scheme).

Fix #118

@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2022

Codecov Report

Merging #126 (bd3bef8) into main (a85c098) will increase coverage by 0.17%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #126      +/-   ##
==========================================
+ Coverage   88.77%   88.94%   +0.17%     
==========================================
  Files          13       13              
  Lines         561      570       +9     
==========================================
+ Hits          498      507       +9     
  Misses         63       63              
Impacted Files Coverage Δ
src/utils.jl 90.90% <100.00%> (+0.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment on lines +143 to +146
batchseq([ones(2,4), zeros(2, 3), ones(2,2)]) ==[[1.0 0.0 1.0; 1.0 0.0 1.0]
[1.0 0.0 1.0; 1.0 0.0 1.0]
[1.0 0.0 0.0; 1.0 0.0 0.0]
[1.0 0.0 0.0; 1.0 0.0 0.0]]

Choose a reason for hiding this comment

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

this behaviour looks good to me (author of #118) - thanks 👍

@CarloLucibello CarloLucibello merged commit 855f95b into main Oct 28, 2022
@CarloLucibello CarloLucibello deleted the cl/pad branch December 28, 2022 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

batchseq for sequences of matrices
3 participants