search

ANTsX / ANTsRCore

Rcpp bindings for the C++ ANTs library used by the ANTsR package
9 stars 9 forks source link

ENH: Range and summaries in C++ #86

Open muschellij2 opened 5 years ago

muschellij2 commented 5 years ago

So we have all the overloaded summary functions in R: sum min, range, etc. These take in a mask, grab the values (in R!) and then compute the statistic. I think it'd be probably loads faster to grab most of these with the antsImage object passed through.

@jeffduda What do you think as you did a bunch for the arithmetic?

jeffduda commented 5 years ago

I definitely think these would be much faster if they are implemented as C++ functions. I'll take a look tomorrow and/or next week. It should be pretty easy to switch the implementation.

On Thu, Jun 6, 2019 at 6:53 PM John Muschelli notifications@github.com wrote:

So we have all the overloaded summary functions in R: sum min, range, etc. These take in a mask, grab the values (in R!) and then compute the statistic. I think it'd be probably loads faster to grab most of these with the antsImage object passed through.

@jeffduda https://github.com/jeffduda What do you think as you did a bunch for the arithmetic?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAD5NCGZG7LNQ3TKLTFA7F3PZGIN7A5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYEMASA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD5NCEZADZQHATAC4S6CB3PZGIN7ANCNFSM4HVLS6OQ .

jeffduda commented 5 years ago

Also, I'm assuming we want to maintain the current style of returning sum/mean/etc of all values for multi-channel images (as opposed to sum/mean/etc of all pixels)? -Jeff

On Thu, Jun 6, 2019 at 8:10 PM Jeffrey Duda jeff.duda@gmail.com wrote:

I definitely think these would be much faster if they are implemented as C++ functions. I'll take a look tomorrow and/or next week. It should be pretty easy to switch the implementation.

On Thu, Jun 6, 2019 at 6:53 PM John Muschelli notifications@github.com wrote:

So we have all the overloaded summary functions in R: sum min, range, etc. These take in a mask, grab the values (in R!) and then compute the statistic. I think it'd be probably loads faster to grab most of these with the antsImage object passed through.

@jeffduda https://github.com/jeffduda What do you think as you did a bunch for the arithmetic?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAD5NCGZG7LNQ3TKLTFA7F3PZGIN7A5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYEMASA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD5NCEZADZQHATAC4S6CB3PZGIN7ANCNFSM4HVLS6OQ .

muschellij2 commented 5 years ago

I would guess so as to not break any behavior relied upon currently.

John

On Fri, Jun 7, 2019 at 5:15 PM Jeffrey Duda notifications@github.com wrote:

Also, I'm assuming we want to maintain the current style of returning sum/mean/etc of all values for multi-channel images (as opposed to sum/mean/etc of all pixels)? -Jeff

On Thu, Jun 6, 2019 at 8:10 PM Jeffrey Duda jeff.duda@gmail.com wrote:

I definitely think these would be much faster if they are implemented as C++ functions. I'll take a look tomorrow and/or next week. It should be pretty easy to switch the implementation.

On Thu, Jun 6, 2019 at 6:53 PM John Muschelli notifications@github.com wrote:

So we have all the overloaded summary functions in R: sum min, range, etc. These take in a mask, grab the values (in R!) and then compute the statistic. I think it'd be probably loads faster to grab most of these with the antsImage object passed through.

@jeffduda https://github.com/jeffduda What do you think as you did a bunch for the arithmetic?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAD5NCGZG7LNQ3TKLTFA7F3PZGIN7A5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYEMASA , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAD5NCEZADZQHATAC4S6CB3PZGIN7ANCNFSM4HVLS6OQ

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAIGPLRECQABXQI53GWUQS3PZLFW3A5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXHAM5I#issuecomment-500041333, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIGPLR5HAAAL6NLQ6XO4ELPZLFW3ANCNFSM4HVLS6OQ .

jeffduda commented 5 years ago

just put in a PR to attempt to address this issue

On Sat, Jun 8, 2019 at 11:31 AM John Muschelli notifications@github.com wrote:

I would guess so as to not break any behavior relied upon currently.

John

On Fri, Jun 7, 2019 at 5:15 PM Jeffrey Duda notifications@github.com wrote:

Also, I'm assuming we want to maintain the current style of returning sum/mean/etc of all values for multi-channel images (as opposed to sum/mean/etc of all pixels)? -Jeff

On Thu, Jun 6, 2019 at 8:10 PM Jeffrey Duda jeff.duda@gmail.com wrote:

I definitely think these would be much faster if they are implemented as C++ functions. I'll take a look tomorrow and/or next week. It should be pretty easy to switch the implementation.

On Thu, Jun 6, 2019 at 6:53 PM John Muschelli < notifications@github.com> wrote:

So we have all the overloaded summary functions in R: sum min, range, etc. These take in a mask, grab the values (in R!) and then compute the statistic. I think it'd be probably loads faster to grab most of these with the antsImage object passed through.

@jeffduda https://github.com/jeffduda What do you think as you did a bunch for the arithmetic?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <

https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAD5NCGZG7LNQ3TKLTFA7F3PZGIN7A5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GYEMASA

,

or mute the thread <

https://github.com/notifications/unsubscribe-auth/AAD5NCEZADZQHATAC4S6CB3PZGIN7ANCNFSM4HVLS6OQ

.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub < https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAIGPLRECQABXQI53GWUQS3PZLFW3A5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXHAM5I#issuecomment-500041333 , or mute the thread < https://github.com/notifications/unsubscribe-auth/AAIGPLR5HAAAL6NLQ6XO4ELPZLFW3ANCNFSM4HVLS6OQ

.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ANTsX/ANTsRCore/issues/86?email_source=notifications&email_token=AAD5NCF2MVB5F6ZVWZ3GDZ3PZPGDJA5CNFSM4HVLS6O2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODXHWY7A#issuecomment-500132988, or mute the thread https://github.com/notifications/unsubscribe-auth/AAD5NCHDMIHELNHNBQTMZGDPZPGDJANCNFSM4HVLS6OQ .