astropy / ccdproc

Astropy affiliated package for reducing optical/IR CCD data
https://ccdproc.readthedocs.io
BSD 3-Clause "New" or "Revised" License
89 stars 87 forks source link

Overscan and Trim Regions in `ccdproc` Shapes Provide ValueError #804

Closed kjkoeller closed 1 year ago

kjkoeller commented 1 year ago

Description

I am attempting to simply subtract an overscan region using the ccd_process function with a ccd, oscan, trim, gain, and readnoise. However, if I allow the oscan to have a limit on the rows that it uses (i.e. '[2073:2115, 12:2057]') and use the same rows for the trim region, then the program gives this error

Traceback (most recent call last):
  File "C:\Users\Kyle\OneDrive\PhysicsAstro\Astronomy\Code\IRAF\IRAF_Reduction.py", line 507, in <module>
    main()
  File "C:\Users\Kyle\OneDrive\PhysicsAstro\Astronomy\Code\IRAF\IRAF_Reduction.py", line 85, in main
    zero, overscan_region, trim_region = bias(files, calibrated_data, path)
  File "C:\Users\Kyle\OneDrive\PhysicsAstro\Astronomy\Code\IRAF\IRAF_Reduction.py", line 244, in bias
    new_ccd = reduce(ccd, overscan_region, trim_region, 0, zero=None, combined_dark=None, good_flat=None)
  File "C:\Users\Kyle\OneDrive\PhysicsAstro\Astronomy\Code\IRAF\IRAF_Reduction.py", line 145, in reduce
    new_ccd = ccdp.ccd_process(ccd, oscan=overscan_region, trim=trim_region, gain=gain * u.electron / u.adu,
  File "C:\Users\Kyle\AppData\Local\Programs\Python\Python39\lib\site-packages\ccdproc\log_meta.py", line 97, in wrapper
    result = func(*args, **kwd)
  File "C:\Users\Kyle\AppData\Local\Programs\Python\Python39\lib\site-packages\ccdproc\core.py", line 201, in ccd_process
    nccd = subtract_overscan(nccd, fits_section=oscan,
  File "C:\Users\Kyle\AppData\Local\Programs\Python\Python39\lib\site-packages\ccdproc\log_meta.py", line 97, in wrapper
    result = func(*args, **kwd)
  File "C:\Users\Kyle\AppData\Local\Programs\Python\Python39\lib\site-packages\ccdproc\core.py", line 493, in subtract_overscan
    subtracted.data = ccd.data - oscan
ValueError: operands could not be broadcast together with shapes (2058,2116) (2046,1) 

I am not even sure where the shapes are coming from as I have tried to get the shape using .shape() for the ccd.

Expected behavior

The expected behavior is for the program not to crash at all and just subtract the overscan and gain correct the ccd image.

How to Reproduce

The code I am running specifically is

for ccd, file_name in files.ccds(imagetyp='BIAS', return_fname=True, ccd_kwargs={'unit': 'adu'}):
      new_ccd = reduce(ccd, overscan_region, trim_region, 0, zero=None, combined_dark=None, good_flat=None)

which calls this line

new_ccd = ccdp.ccd_process(ccd, oscan=overscan_region, trim=trim_region, gain=gain * u.electron / u.adu,
                                   readnoise=rdnoise * u.electron, error=False)

with the gain value being 1.43 and readnoise being 10.83. I am reading in the CCD with the units of ADU.

Versions

Python version: 3.9 Astropy version: 5.1.1 have also tried 5.2.2 Numpy version: 1.22.3 Pyerfa version: 2.0.0.1 Scipy version: 1.9.3 Matplotlib version: 3.5.1

github-actions[bot] commented 1 year ago

Welcome to Astropy 👋 and thank you for your first issue!

A project member will respond to you as soon as possible; in the meantime, please double-check the guidelines for submitting issues and make sure you've provided the requested details.

GitHub issues in the Astropy repository are used to track bug reports and feature requests; If your issue poses a question about how to use Astropy, please instead raise your question in the Astropy Discourse user forum and close this issue.

If you feel that this issue has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail feedback@astropy.org.

pllim commented 1 year ago

Hi, I transferred this issue to ccdproc repo.

I am not familiar with the package but at a glance, I think you need to compute the overscan value using the overscan region first, so your data array would subtract a scalar. Usually it is np.mean or np.median, depending on the behavior of your instrument.

pllim commented 1 year ago

But if you are following the official doc (say, https://ccdproc.readthedocs.io/en/latest/reduction_toolbox.html#subtract-overscan-and-trim-images) and encountered this error, then this is a bug.

kjkoeller commented 1 year ago

Thank you @pllim and I have also attempted the individual 'subtract_overscan' and 'trim_image' as well but results in the same the same error with weird shapes trying to be subtracted. I might also add that the image is essentially 2kx2k with the overscan on the right side of image and the values of columns increasing from left to right and rows increasing from top to bottom.

pllim commented 1 year ago

Without a fully reproducible code provided, hard to say, but there is dimension mismatch in your trimming somewhere, the error message says (2058,2116) (2046,1).

You need to make them either (2046,2116) (2046,1) or (2058,2116) (2058,1), I think.

kjkoeller commented 1 year ago

I have the full code uploaded on GitHub at this page (https://github.com/kjkoeller/EclipsingBinaries/blob/main/EclipsingBinaries/IRAF_Reduction.py) with example images located on GitHub at this page (https://github.com/kjkoeller/EclipsingBinaries/tree/main/examples/calibration_images). Hope this helps anyone and the overscan region, when prompted, should be [2073:2115, 12:2057] and the trim data section (trim region), when prompted, should be [20:2060, 12:2057]. This will reproduce what I am doing.

pllim commented 1 year ago

Wow, that trim stuff is super confusing (FITS convention!): https://github.com/astropy/ccdproc/blob/623162d4f32d13e2153e97a91bade0cf9de074de/ccdproc/utils/slices.py#L20

cc @mwcraig

kjkoeller commented 1 year ago

So you are suggesting to have fits_convention variable added to the ccd_process?

Edit: There is no argument for fits_convention in ccd_process

pllim commented 1 year ago

@kjkoeller , sorry, that was just a commentary on how confusing it is to use FITS convention in Python code. It is not the solution. I pinged the maintainer hoping for more insight into this problem. Thanks for your patience!

kjkoeller commented 1 year ago

I might have to do the step manually and enter a fits_section to the subtract_bias function rather than using ccd_process https://github.com/astropy/ccdproc/blob/623162d4f32d13e2153e97a91bade0cf9de074de/ccdproc/core.py#L385

Also thank you for taking time out of your day to try to find a solution!

Edit: I have attempted the following line with the same error. ccd = ccdp.subtract_overscan(ccd, fits_section=overscan_region, median=True, overscan_axis=None)

kjkoeller commented 1 year ago

Following up for any updates @mwcraig or @pllim?

pllim commented 1 year ago

Alas, I don't maintain this package. I simply moved this issue from astropy to ccdproc. Were you not able to make it work by using lower level functions?

kjkoeller commented 1 year ago

Very sorry to ping you then and no I was not. Still having the same errors come up no matter how I order the values or what I try.

mwcraig commented 1 year ago

Thanks for reporting the bug @kjkoeller. At the latest, I'll have time to look at this Monday of next week. I'll try to get to it before then.

kjkoeller commented 1 year ago

I appreciate the help whenever you get the opportunity.

kjkoeller commented 1 year ago

@mwcraig Have you had a chance to look at the bug?

mwcraig commented 1 year ago

@kjkoeller -- sorry for the delay. Thanks for providing the links to your repo -- that is very helpful for reproducing the issue.

I'm taking a look tonight and think I see the issue (and have a fix) after looking at one of the images to get a clearer picture where the overscan regions are. In ccd_process, overscan subtraction is done before trimming. I think the overscan region for your images should be [2073:2115, :] -- I gather the region [:, 0:20] isn't meaningful (it shows up as a black bar on the image) but you shouldn't exclude from the overscan. When you do, the overscan ends up as 2046 rows (the (2046, 1) in the traceback) and then ccdproc tries to subtract that from an image that still has 2116 rows (the original image dimension).

That result in an error because the overscan has a different number of rows than the image.

If you set the overscan region to [2073:2115, :] and then trim to [20:2060, 12:2057] I think you end up with the output you want.

Incidentally, the EclipsingBinary repo looks interesting -- I may be in touch over the summer with you and a couple other folks working on similar things to see if we can maybe pool/share dcode.

kjkoeller commented 1 year ago

Thanks for taking a look! It is much appreciated, and I actually used to have the inputs be [2073:2115, :] but I have run into weird behavior where the master bias is about 20 counts higher than what the old Linux IRAF produces. I thought a possible cause of this was due to the difference in dimensions between the overscan and trim sections.

However, from your response it seems that, that is not he case actually and there is something else at play causing these issues.

Also I look forward to collaborating with you and others if that collaboration happens.

mwcraig commented 1 year ago

@kjkoeller -- if you can send an example where the bias ends up higher I'd be happy to take a look

kjkoeller commented 1 year ago

It's actually only like 10 counts higher between the master bias images, but for the master flats there is a 12k-20k count difference (more counts in the Python version vs. the Linux version). I have uploaded the master files here (GitHub page). The other weird thing is, is that the file size between the Linux version and Python. The Linux version is about 4x smaller than the Python corresponding versions.

I am not sure if this is supposed to happen either, but when I open the fits files that are outputted from the Python version, it shows three separate images within the master flat file.

The first image is the master flat, the second image is an image of all 0 count pixel values, and the third is not even a single flat image but maybe a mask?

Screenshot 2023-05-08 100510 Screenshot 2023-05-08 100532 Screenshot 2023-05-08 100552

kjkoeller commented 1 year ago

@mwcraig I think I will open a new issue request as the original issue has been resolved and now we are on to a different topic. If that is okay with you, I will post the same images as my last post.

mwcraig commented 1 year ago

I'm going to close this @kjkoeller since you have opened a separate issue for the flat field thing. If you want to reopen this issue please feel free to.