Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Context Maps #239

Merged
merged 1 commit into from
Oct 8, 2024
Merged

Support for Context Maps #239

merged 1 commit into from
Oct 8, 2024

Conversation

TheSkyentist
Copy link
Contributor

The current grizli implementation of AstroDrizzle was not correctly computing context maps, since it was always setting the ID of the input image to 1, rather than incrementing it. This was fixed in 77412f6. I've also added the ability to optionally output context maps when creating mosaics for the direct and grism images.

@TheSkyentist
Copy link
Contributor Author

A quick note, AstroDrizzle does not support more than 32 images in a drizzle. So if this happens, print a warning and continue.

@gbrammer
Copy link
Owner

gbrammer commented Sep 2, 2024

Finally getting around to this. Could you rename the parameter controlling whether to output the context image out_ctx to write_context?

And for the issue with more than 32 images, how about just wrapping the integer at 32? Since drizzle_array_groups is effectively calculating the context anyway, you don't need to add logic whether or not to do it, drizzle_array_groups can have the parameter default drizzle_array_groups(first_uniqid=1) and then inside adrizzle.do_driz(..., uniqid=((first_uniqid - 1 + i) % 32) + 1).

Finally, it looks like there are some debugging lines that are commented out and should be removed for clarity.

Thanks!

@TheSkyentist
Copy link
Contributor Author

TheSkyentist commented Sep 2, 2024 via email

@TheSkyentist
Copy link
Contributor Author

I've addressed the handling of ID assignment, as well as made the warning log use the grizli msg.log syntax. In addition, I properly implemented the in-progress sections that were commented out.

@gbrammer
Copy link
Owner

gbrammer commented Sep 5, 2024

Looking good.

  • There seems to still be one commented block in aws_drizzler
  • Can you rename ctx_out to something a bit more meaningful (e.g., write_ctx_file, write_context)

@TheSkyentist
Copy link
Contributor Author

Good suggestions, I've reformatted to write_ctx and fixed the last comment block.

@TheSkyentist
Copy link
Contributor Author

I've updated this commit to be merge-able after the most recent changes.

@gbrammer gbrammer merged commit 71b8167 into gbrammer:master Oct 8, 2024
17 checks passed
@TheSkyentist TheSkyentist deleted the ctx branch October 8, 2024 09:12
@TheSkyentist TheSkyentist restored the ctx branch October 9, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants