modify the _embed function to fit the 2d input#15
modify the _embed function to fit the 2d input#15raphaelvallat merged 3 commits intoraphaelvallat:masterfrom cheliu-computation:patch-2
Conversation
modify the _embed funciton, so, it can take input as 2d array. pre store the sliced signal into a list to accelerate concatenation operation. pre define the indice of sliced signal to reduce the computing in loop. add vectorized operation in loop to slice the signal for all input signal in one time. performance: 1e3 signal with 1000 time point, order = 3, decay=1: 0.01s 1e4 signal with 1000 time point, order = 3, decay=1: 0.1s 1e4 signal with 1000 time point, order = 10, decay=1: 0.85s 1e5 signal with 1000 time point, order = 3, decay=1: 1.11s 1e5 signal with 1000 time point, order = 10, decay=1: 9.82s 5e5 signal with 1000 time point, order = 3, decay=1: 67s
|
Thanks for opening the PR. I have been wanting to implement a 2D version of this function for a long time. I do have several comments however:
Thanks, |
|
Thanks for your review.
While the original funciton cannot do this directly. If I use some numpy API, such as np.concatenate, it will have O(n) time complexity and O(2n) space complexity. Apperently that's unfair to test two functions. I have tested two functions with 2D array without the assemble operation (no concatenate, stack, etc...) The modified function only has O(1) on time and O(n) on space. Is that an acceptable way to compare two functions? |
add one condition to detect the array dimension if 1d array, follow the original function code if 2d array, return 3d array (signals_number, embedded signal length, order)
add one condition to detect the array dimension if 1d array, follow the original function code if 2d array, return 3d array (signals_number, embedded signal length, order)
|
Hi , I have fixed the function to be able pass 1d and 2d array both. The return would be 2d if you pass 1d as the original function. I think there is no problem here. Hoever, if you pass 2d array, the function will return 3d array and no function from 'antropy' can deal with it so far, becasue I have not modified other functions. Do you prefer to merge it after I have modified other entropy computing function to be able pass the result (3d array) from the modified function? It might takes lots of time to modified whole function in this package. |
|
Thank you @cheliu-computation. I think we can merge this first PR and then you, me and/or others can work on adding support for such 3D arrays in the other functions. |
|
Thanks for your work. Is my commitment can be merged now or need more modification? I have seen the test has been passed.
Best Regards
Che Liu
…________________________________
From: Raphael Vallat ***@***.***>
Sent: Friday, January 21, 2022 1:05:15 AM
To: raphaelvallat/antropy ***@***.***>
Cc: Che Liu ***@***.***>; Mention ***@***.***>
Subject: Re: [raphaelvallat/antropy] modify the _embed function to fit the 2d input (PR #15)
Thank you @cheliu-computation<https://github.com/cheliu-computation>. I think we can merge this first PR and then you, me and/or others can work on adding support for such 3D arrays in the other functions.
—
Reply to this email directly, view it on GitHub<#15 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ANUMLRXBBJEEU3H3UMB7FQLUXCWMXANCNFSM5MK7SKFA>.
Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Merging now! |
2 participants
modify the _embed funciton, so, it can take input as 2d array.
pre store the sliced signal into a list to accelerate concatenation operation.
pre define the indice of sliced signal to reduce the computing in loop.
add vectorized operation in loop to slice the signal for all input signal in one time.
performance:
1e3 signal with 1000 time point, order = 3, decay=1: 0.01s
1e4 signal with 1000 time point, order = 3, decay=1: 0.1s
1e4 signal with 1000 time point, order = 10, decay=1: 0.85s
1e5 signal with 1000 time point, order = 3, decay=1: 1.11s
1e5 signal with 1000 time point, order = 10, decay=1: 9.82s
5e5 signal with 1000 time point, order = 3, decay=1: 67s