grizli icon indicating copy to clipboard operation
grizli copied to clipboard

Support for Context Maps

Open TheSkyentist opened this issue 1 year ago • 6 comments

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 avatar Jul 20 '24 08:07 TheSkyentist

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

TheSkyentist avatar Jul 24 '24 12:07 TheSkyentist

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!

gbrammer avatar Sep 02 '24 08:09 gbrammer

This is great feedback, I’ll get this implemented soon.

On Mon, Sep 2, 2024 at 10:05 Gabe Brammer @.***> wrote:

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!

— Reply to this email directly, view it on GitHub https://github.com/gbrammer/grizli/pull/239#issuecomment-2324080647, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEB6FNGRSK27NK45VJSJCUTZUQL5VAVCNFSM6AAAAABLFYG342VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMRUGA4DANRUG4 . You are receiving this because you authored the thread.Message ID: @.***>

TheSkyentist avatar Sep 02 '24 14:09 TheSkyentist

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.

TheSkyentist avatar Sep 04 '24 08:09 TheSkyentist

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)

gbrammer avatar Sep 05 '24 10:09 gbrammer

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

TheSkyentist avatar Sep 05 '24 10:09 TheSkyentist

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

TheSkyentist avatar Oct 07 '24 18:10 TheSkyentist