Skip to content

Add SIA access for TDS images in TDE light curve notebook#14

Draft
jaladh-singhal wants to merge 7 commits intoCaltech-IPAC:mainfrom
jaladh-singhal:GW_host-SIA-updates
Draft

Add SIA access for TDS images in TDE light curve notebook#14
jaladh-singhal wants to merge 7 commits intoCaltech-IPAC:mainfrom
jaladh-singhal:GW_host-SIA-updates

Conversation

@jaladh-singhal
Copy link
Member

Fixes #13

@jaladh-singhal jaladh-singhal self-assigned this Feb 25, 2026
@jaladh-singhal jaladh-singhal added the content: GW_host Gravitational wave use case label Feb 25, 2026
Copy link
Member Author

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

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

I have a bunch of questions/concerns

#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
Copy link
Member Author

Choose a reason for hiding this comment

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

@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?

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

@bsipocz what is a reasonable time for a tutorial to run? I vaguely recall hearing 2 - 3 minutes

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also I do feel, it can be easily parallelized for each candidate since they are independent operations.

Copy link
Contributor

@jkrick jkrick left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @jaladh-singhal

#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
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Collaborator

@troyraen troyraen left a comment

Choose a reason for hiding this comment

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

Looks like a good improvement to me. I see you're working out some details with @jkrick. I defer to both of you for those.

@jkrick
Copy link
Contributor

jkrick commented Mar 3, 2026

@jaladh-singhal Is this PR ready for me to review?

@jaladh-singhal
Copy link
Member Author

jaladh-singhal commented Mar 3, 2026

@jaladh-singhal Is this PR ready for me to review?

@jkrick Umm...no. I realised:

  1. My TDS search radius was too big. Since you already identified galaxy candidates, I should have chosen a very narrow region on the galaxy center. That reduces number of TDS images returned.
  2. The code was reading the FITS header of each TDS image file for finding MJD which consumes time because of I/O with cloud. SIA results return mjd columns, I think I can use that instead.

I'm marking it as draft. Will mark it ready when I address above.

@jaladh-singhal jaladh-singhal marked this pull request as draft March 3, 2026 20:02
Other improvements in TDE info extraction
@jaladh-singhal jaladh-singhal changed the title Add SIA access for TDS images in GW_host notebook Add SIA access for TDS images in TDE light curve notebook Mar 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

content: GW_host Gravitational wave use case

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add SIA access for TDS images in TDE notebook

4 participants