tlambert03 / pycudadecon

Python wrapper for cudaDecon - GPU accelerated 3D deconvolution for microscopy
http://www.talleylambert.com/pycudadecon/
MIT License
59 stars 12 forks source link

Task: implement decon-before-deskew #43

Closed tlambert03 closed 1 month ago

tlambert03 commented 1 year ago

tracking support for https://github.com/scopetools/cudadecon/pull/23 on the python side

zichenzachwang commented 10 months ago

Hi Talley,

I noticed that in 0.4.1 release you mentioned there is added support for sample-scan decon before deskew. I recently tried adding skewed_decon=True with 3D sample-scan PSF acquired on LLSM. However, the kernel crashed after I tried. I have tested with cudaDecon and it worked. I am wondering if this is some functionality you have already added or still in development. I can provide more information if it is supposed to work.

Zach

tlambert03 commented 10 months ago

hey @zichenzachwang. I think @dmilkie did expose support for skewed-decon https://github.com/tlambert03/pycudadecon/pull/47. I admit I haven't had time to try it yet, perhaps he can weigh in? @dmilkie, have you been using skewed decon via pycudadecon successfully? Does it still need tweaking? If anyone has or could generate a very small test dataset that we could add to test_data that would be a nice bonus :)

then we can discuss the kernel crash that @zichenzachwang is observing

dmilkie commented 10 months ago

I thought it was as simple as adding support for thre flag, but it was a little more than that. I’m in the middle of confirming it’s working. I need to double check that the OTF scaling (with OTF voxel size) is working properly before pushing it up. I have not seen a “kernel crash”. What is the error message?-DanOn Nov 3, 2023, at 10:38 AM, Talley Lambert @.***> wrote: hey @zichenzachwang. I think @dmilkie did expose support for skewed-decon #47. I admit I haven't had time to try it yet, perhaps he can weigh in? @dmilkie, have you been using skewed decon via pycudadecon successfully? Does it still need tweaking? If anyone has or could generate a very small test dataset that we could add to test_data that would be a nice bonus :) then we can discuss the kernel crash that @zichenzachwang is observing

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

zichenzachwang commented 10 months ago

Hi guys thanks for the help! I took a closer look at both repositories and noticed something. In cudaDecon #23, radialft_interface.cpp was not get changed the the same way Linshao did for radialft-nonSIM.cpp. That way the libradialft in pycudadecon is not updated (need b3Dout arg and some fix_origin modifications). Also I think in pycudadecon _libwrap.py and otf.py needs to add support for 3D OTF. I rebuilt cudaDecon and it seems to work very well. Let me know if you think this is the issue. I am happy to share my modified codes if it helps.

zichenzachwang commented 10 months ago

I thought it was as simple as adding support for thre flag, but it was a little more than that. I’m in the middle of confirming it’s working. I need to double check that the OTF scaling (with OTF voxel size) is working properly before pushing it up. I have not seen a “kernel crash”. What is the error message?-DanOn Nov 3, 2023, at 10:38 AM, Talley Lambert @.> wrote: hey @zichenzachwang. I think @dmilkie did expose support for skewed-decon #47. I admit I haven't had time to try it yet, perhaps he can weigh in? @dmilkie, have you been using skewed decon via pycudadecon successfully? Does it still need tweaking? If anyone has or could generate a very small test dataset that we could add to test_data that would be a nice bonus :) then we can discuss the kernel crash that @zichenzachwang is observing —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.>

I am using jupyter notebook on Ubuntu and it wasn't clear to me. No explicit error message and certainly not memory issue.

dmilkie commented 10 months ago

I (probably) did the same thing.
https://github.com/dmilkie/cudaDecon/commit/ff7a039a994a667112ea3a484e9431510cce4ee9 https://github.com/dmilkie/pycudadecon/commit/ca52c146a7406f9c518e95254e9a2c354a407191 My current state of code is on my two branches there.

zichenzachwang commented 10 months ago

I (probably) did the same thing. dmilkie/cudaDecon@ff7a039 dmilkie@ca52c14 My current state of code is on my two branches there.

Right that will be great! I couldn't build it myself on Windows though so I will probably wait for your official release. Thanks a lot.

zichenzachwang commented 9 months ago

I (probably) did the same thing. dmilkie/cudaDecon@ff7a039 dmilkie@ca52c14 My current state of code is on my two branches there.

Hi Dan, I am curious how your tests went. Do you think you may release new versions of cudadeon and pycudadecon with functional sample stage decon sometime soon? That would be very helpful thx! Zach

dmilkie commented 9 months ago

It's still on my todo list. Hopefully get to it next week.

zichenzachwang commented 6 months ago

Hi Dan,

Just a reminder to see if you will be able to fix this issue and update pycudadecon soon. Thanks!

Zach


From: Dan Milkie @.> Sent: Tuesday, November 28, 2023 11:24 To: tlambert03/pycudadecon @.> Cc: Zachary Wang @.>; Mention @.> Subject: Re: [tlambert03/pycudadecon] Task: implement decon-before-deskew (Issue #43)

It's still on my todo list. Hopefully get to it next week.

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https://github.com/tlambert03/pycudadecon/issues/43*issuecomment-1830530019__;Iw!!Mih3wA!Fnu9NVoNA541_aXz3XoRLtpf3JYiUvz_isPCt_zl8rsmbV7nNv_QgeMch_symRPzbuvpaCnMvtkE7_tVQploj22bsr0$, or unsubscribehttps://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/ARHD2BWYDPOFJHY6G4Y55S3YGY25LAVCNFSM6AAAAAASZL7LSOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMZQGUZTAMBRHE__;!!Mih3wA!Fnu9NVoNA541_aXz3XoRLtpf3JYiUvz_isPCt_zl8rsmbV7nNv_QgeMch_symRPzbuvpaCnMvtkE7_tVQplodzhINdI$. You are receiving this because you were mentioned.Message ID: @.***>

tlambert03 commented 1 month ago

sounds like this is now complete thanks to @zichenzachwang's work in #57 and elsewhere, correct @zichenzachwang ?

zichenzachwang commented 1 month ago

sounds like this is now complete thanks to @zichenzachwang's work in #57 and elsewhere, correct @zichenzachwang ?

Yes! Tested on my end and works well.