Add SIA access for TDS images in TDE light curve notebook#14
Add SIA access for TDS images in TDE light curve notebook#14jaladh-singhal wants to merge 7 commits intoCaltech-IPAC:mainfrom
Conversation
jaladh-singhal
left a comment
There was a problem hiding this comment.
I have a bunch of questions/concerns
tutorials/GW_host.md
Outdated
| #for each candidate galaxy: | ||
| for idx, row in df_candidates.iterrows(): | ||
| filenames = row["image_filenames"] | ||
| filenames = row["image_filenames"][:10] # limit to first 10 images for speed; fix later |
There was a problem hiding this comment.
@jkrick the number of simple model TDS images for each candidate are ~600-700. I picked first 10 because I didn't know anything better. What should we do about it to keep execution time in check? Parallelize photometry or something else?
There was a problem hiding this comment.
good question, I hadn't considered this. I don't know how long these things are taking to run. If possible to parallelize and keep under a reasonable time, that would be great, if not limit the number to something that takes a reasonable amount of time? Also ok if you want to push parallelization to a separate issue/PR to not do too much in this one, that is fine by me.
There was a problem hiding this comment.
It took my local machine (M1 Pro, 32GB) 4m26s for performing photometry on 4x10 images. What's a "a reasonable time" we should aim for?
And I agree, parallelizing this is beyond the scope of this PR and should be tackled in a separate ticket.
There was a problem hiding this comment.
@bsipocz what is a reasonable time for a tutorial to run? I vaguely recall hearing 2 - 3 minutes
There was a problem hiding this comment.
Well, it depends. I think anything above a minute gets tedious unless you plan to fill the time with something else. E.g. you start a cell then go to a different tab and explain something else while the cell is running. That way it's reasonable to go up to ~5mins?
(The way I think about tutorials is to learn techniques, datasets, etc, and not necessarily do publishing ready science -- e.g. the audience should totally expect that shortcuts are being made and that they need to run the same thing on orders of magnitude more sources etc.)
And is a subset is not meaningful to showcase the rest of a tutorial, reusing some canned partial results is usually acceptable, but I would still recommend to do the whole thing with just a smaller sample whenever possible.
There was a problem hiding this comment.
Ok then, I will only keep 9 (or less) images representative of their mjd quantiles here too - same as what @jkrick suggested for cutout gallery.
There was a problem hiding this comment.
Also I do feel, it can be easily parallelized for each candidate since they are independent operations.
jkrick
left a comment
There was a problem hiding this comment.
Thanks for working on this @jaladh-singhal
tutorials/GW_host.md
Outdated
| #for each candidate galaxy: | ||
| for idx, row in df_candidates.iterrows(): | ||
| filenames = row["image_filenames"] | ||
| filenames = row["image_filenames"][:10] # limit to first 10 images for speed; fix later |
There was a problem hiding this comment.
good question, I hadn't considered this. I don't know how long these things are taking to run. If possible to parallelize and keep under a reasonable time, that would be great, if not limit the number to something that takes a reasonable amount of time? Also ok if you want to push parallelization to a separate issue/PR to not do too much in this one, that is fine by me.
|
@jaladh-singhal Is this PR ready for me to review? |
@jkrick Umm...no. I realised:
I'm marking it as draft. Will mark it ready when I address above. |
ff0b147 to
d11244f
Compare
… adjust aperture radius handling
d11244f to
b8cab21
Compare
Other improvements in TDE info extraction
Fixes #13