soar-telescope / goodman_pipeline

Goodman Data Reduction Pipeline
BSD 3-Clause "New" or "Revised" License
15 stars 13 forks source link

Combine Images for Improving SN #60

Open simontorres opened 7 years ago

simontorres commented 7 years ago

Add posibility to combine multiple exposure to improve the Signal to noise ratio. This should be done in the ccd reduction part. It should also possible to get the processed single images.

cbaorion commented 7 years ago

Yes, specially true for imaging. But people may want to combine the extracted, 1-D spectra instead of the raw ones. This can be done after the processing. I would not spend too much time adding this feature. I don think this is needed for the beta release.

simontorres commented 4 years ago

There is some code already in place, the thing is in which part of the process the combination should happen.

  1. Raw, before any processing
  2. After redccd has done all what it does currently
  3. As @cbaorion said, someone might want to combine the 1D spectra, I suppose this requires that the 1D spectra were extracted using the same aperture width (does it really matters?)

Also, I'm wondering whether we should assume that the spectra are well aligned or include some 3d-matching/align routine.

simontorres commented 4 years ago

The routine core.combine_data is working actually quite nice. One of the things that I could use a suggestion is how to name the files. There could be one schema or a couple of predefined ones.

Another problem I just found is when combining a lamp-bracketed target is that the lamps from before and after are shifted with respect to each other, which is a possibility, but this means they need to be combined separately. The problem remains to find the appropriate identifiers.

rachel3834 commented 4 years ago

To keep within the pipeline's existing system of prepending prefix letters, perhaps use the letter 'k'(ombined)? Since 'c' is already assigned of course. Or to make it really obvious, add the suffix '_combo'. I would suggest taking the file number out of the filename, since it would refer to multiple files, but the files used in the combination should be listed in the FITS header.

I agree if there is a shift in the bracketed lamps, separate combinations will be needed, yes. In that case, I'd suggest a number suffix. Hope this helps!

On Wed, Apr 8, 2020 at 12:40 PM Simon Torres notifications@github.com wrote:

The routine core.combine_data is working actually quite nice. One of the things that I could use a suggestion is how to name the files. There could be one schema or a couple of predefined ones.

Another problem I just found is when combining a lamp-bracketed target is that the lamps from before and after are shifted with respect to each other, which is a possibility, but this means they need to be combined separately. The problem remains to find the appropriate identifiers.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/soar-telescope/goodman_pipeline/issues/60#issuecomment-611153591, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACPJA3HU6YOWVODVCO2JNKLRLTHKLANCNFSM4DKYQMRQ .

simontorres commented 4 years ago

The k is a good idea, but it could be a source of problems for the next stage where the prefix is used for selecting the images to be extracted, in that case we would want to skip double processing the images used for combination but still process the images that did not met the combination requirements, an easy solution would be to run redspec twice but this could turn out confusing for a new user.

Other alternative could be to rename the images used in a combination and add them a prefix like ignore_ and replace the four numeric characters by _comb_,

right now any image resulting from a combination of other gets the keywords that follows the pattern GSP_ICXX where XX represents numerical values, I don't think anyone would combine a 100 pictures so I only left two digits, also all the new files get the suffix _XXX, a zero-padded number. I will experiment on these options and will report later.

rachel3834 commented 4 years ago

Other alternative could be to rename the images used in a combination and add them a prefix like ignore_ and replace the four numeric characters by comb,

For me, adding a prefix of "ignore" could well confuse the user, so maybe a different prefix but replacing the numeric characters with comb makes sense.

right now any image resulting from a combination of other gets the keywords that follows the pattern GSP_ICXX where XX represents numerical values, I don't think anyone would combine a 100 pictures so I only left two digits, also all the new files get the suffix _XXX, a zero-padded number. I will experiment on these options and will report later.

It's always hard to predict what users will do with your code once you let it out into the wild! ;-) But in general yes, I would say you are probably safe with 2 digits.

simontorres commented 4 years ago

Another reason is the 8-character limit on fits keywords. I wanted to have keyword names that make sense. Whether I achieved it or not I don't know but at least they are consistent.

cbaorion commented 4 years ago

Hi Simon, On combining, you always want to combine reduced images. It is not useful to combine raw data. So the combination should be implemented AFTER REDCCD is run. Now, you also want to add the possibility of combining the extracted 1-D spectra. That would require offering the user those two options. Regarding naming, I would use the k as a prefix. I don't quite understand why you say it cannot be used because of confusion latter in the reduction?

simontorres commented 4 years ago

Thanks @cbaorion,

The combination function is ran at the last step of redccd, I tested the naming schema I proposed and so far I think is the best. Adding k to combined images would eventually require changes in redspec or adding K prefix to all other (not combined) images for easy filtering. So, what I've impletented works more or less like this.

Selects the images to be combined, replaces the second 4-digits string by comb (also the function has the option to pass a name of the resulting image). And the images used in the combination get an underscore appended so they are ignored later. One of the problems that I haven't had the chance to solve yet is that all bracketing comparison lamps get combined into a single one.

Below is a sample of files resulting after running redccd --combine @rachel3834 I hope you don't mind I'm using your data for this example.

_cfzsto_0157_Gaia19esl-20-11-2019_comp.fits
_cfzsto_0158_Gaia19esl-20-11-2019_comp.fits
_cfzsto_0159_Gaia19esl-20-11-2019_comp.fits
_cfzsto_0160_Gaia19esl-20-11-2019.fits
_cfzsto_0161_Gaia19esl-20-11-2019.fits
_cfzsto_0162_Gaia19esl-20-11-2019.fits
_cfzsto_0163_Gaia19esl-20-11-2019_comp.fits
_cfzsto_0164_Gaia19esl-20-11-2019_comp.fits
_cfzsto_0165_Gaia19esl-20-11-2019_comp.fits
cfzsto_comb_Gaia19esl-20-11-2019_001.fits
cfzsto_comb_Gaia19esl-20-11-2019_comp_001.fits
dcr.par
master_bias_RED_SP_2x2_R03.89_G01.48.fits
master_flat_400m2_GG455_1.0_night.fits
master_flat_400m2_GG455_1.0_sky.fits
norm_master_flat_400m2_GG455_1.0_sky.fits

The interesting files here are:

cfzsto_comb_Gaia19esl-20-11-2019_001.fits
cfzsto_comb_Gaia19esl-20-11-2019_comp_001.fits

When this is working correctly the result should be something like this:

cfzsto_comb_Gaia19esl-20-11-2019_comp_001.fits
cfzsto_comb_Gaia19esl-20-11-2019_001.fits
cfzsto_comb_Gaia19esl-20-11-2019_comp_002.fits

For AEON nights using OBSID for filtering should work but this will not work on non-AEON nights.

simontorres commented 4 years ago

I uploaded version 1.3.1.dev6 to pypi were I implemented a way to properly combine data at the end of the process of redccd. It requires the use of the argument --combine

I believe this is the best approach for this.

This version can be installed as follows. pip install goodman-pipeline==1.3.1.dev6

rachel3834 commented 4 years ago

Thanks for this update. I installed this version and tried it with data on Gaia19blf taken on 2019-11-20, but I'm seeing the following issue: redccd --combine WARNING: OldEarthOrientationDataWarning: Your version of the IERS Bulletin A is 29.7 days old. For best precision (on the order of arcseconds), you must download an up-to-date IERS Bulletin A table. To do so, run:

from astroplan import download_IERS_A download_IERS_A() [astroplan.utils] [16:01:22][I]: Starting Goodman HTS Pipeline Log [16:01:22][I]: Local Time : 2020-05-01 16:01:22.044428 [16:01:22][I]: Universal Time: 2020-05-01 16:01:22.044624 [16:01:22][W]: Running Development version: 1.3.1.dev6 [16:01:22][I]: Latest Release: 1.3.0 [16:01:22][I]: Found 45 fits files in /data01/rstreet/data_reduction/SOAR/Gaia19blf. [16:01:22][I]: No special reduced data path defined. Proceeding with defaults. [16:01:22][W]: Folder for reduced data is not empty [16:01:22][E]: Please define another folder using --red-path or or use --auto-clean to automatically wipe the current one. (goodman_pipeline) [rstreet@einstore.science.lco.global Gaia19blf]$ mv RED RED2 (goodman_pipeline) [rstreet@einstore.science.lco.global Gaia19blf]$ redccd --combine WARNING: OldEarthOrientationDataWarning: Your version of the IERS Bulletin A is 29.7 days old. For best precision (on the order of arcseconds), you must download an up-to-date IERS Bulletin A table. To do so, run:

from astroplan import download_IERS_A download_IERS_A() [astroplan.utils] [16:01:50][I]: Starting Goodman HTS Pipeline Log [16:01:50][I]: Local Time : 2020-05-01 16:01:50.498977 [16:01:50][I]: Universal Time: 2020-05-01 16:01:50.499074 [16:01:50][W]: Running Development version: 1.3.1.dev6 [16:01:50][I]: Latest Release: 1.3.0 [16:01:50][I]: Found 45 fits files in /data01/rstreet/data_reduction/SOAR/Gaia19blf. [16:01:50][I]: No special reduced data path defined. Proceeding with defaults. [16:01:50][W]: Reduction folder doesn't exist. [16:01:51][I]: Created directory for reduced data. [16:01:51][I]: New directory: /data01/rstreet/data_reduction/SOAR/Gaia19blf/RED [16:01:52][I]: Instrument: Red Camera [16:01:52][I]: Detected Spectroscopy Data from Red Camera [16:01:52][I]: Observing Technique: Spectroscopy [16:01:53][I]: Organizing data for this configuration: Gain: 1.48, Noise: 3.89, ROI: Spectroscopic 2x2 file naxis ... radeg decdeg 15 0169_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.48 16 0170_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.48 17 0171_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.48 18 0172_Gaia19blf-20-11-2019_comp.fits 2 ... 318.74 -39.48 19 0173_Gaia19blf-20-11-2019_comp.fits 2 ... 318.74 -39.48 20 0174_Gaia19blf-20-11-2019_comp.fits 2 ... 318.74 -39.48 21 0175_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.49 22 0176_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.49 23 0177_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.49 24 0178_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 25 0178_Gaia19blf-20-11-2019_comp.fits 2 ... 318.74 -39.48 26 0179_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 27 0179_Gaia19blf-20-11-2019_comp.fits 2 ... 318.74 -39.48 28 0180_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 29 0180_Gaia19blf-20-11-2019_comp.fits 2 ... 318.74 -39.48 30 0181_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 31 0181_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.48 32 0182_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 33 0182_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.48 34 0183_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 35 0183_Gaia19blf-20-11-2019.fits 2 ... 318.74 -39.48 36 0184_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 37 0185_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 38 0186_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 39 0187_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 40 0188_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 41 0189_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 42 0190_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 43 0191_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95 44 0192_DFLAT-03-12-2019.fits 2 ... 129.23 -78.95

[30 rows x 22 columns] Traceback (most recent call last): File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/algorithms.py", line 635, in factorize order = uniques.argsort() TypeError: '<' not supported between instances of 'Undefined' and 'int'

During handling of the above exception, another exception occurred:

Traceback (most recent call last): File "/home/rstreet/miniconda3/envs/goodman_pipeline/bin/redccd", line 17, in GOODMAN_CCD() File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/goodman_pipeline/images/goodman_ccd.py", line 244, in call data_container_list = night_organizer() File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/goodman_pipeline/images/night_organizer.py", line 169, in call data_container=self.data_container) File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/goodman_pipeline/images/night_organizer.py", line 318, in spectroscopy_night 'obsid']).size().reset_index().rename(columns={0: 'count'}) File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 1394, in size result = self.grouper.size() File "/home/rstreet/miniconda3/envs/goodmanpipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 2308, in size ids, , ngroup = self.group_info File "pandas/_libs/properties.pyx", line 36, in pandas._libs.properties.CachedProperty.get File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 2335, in group_info comp_ids, obs_group_ids = self._get_compressed_labels() File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 2351, in _get_compressed_labels all_labels = [ping.labels for ping in self.groupings] File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 2351, in all_labels = [ping.labels for ping in self.groupings] File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 3070, in labels self._make_labels() File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/groupby/groupby.py", line 3103, in _make_labels self.grouper, sort=self.sort) File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/util/_decorators.py", line 177, in wrapper return func(*args, **kwargs) File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/algorithms.py", line 643, in factorize assume_unique=True) File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/sorting.py", line 443, in safe_sort ordered = sort_mixed(values) File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/pandas/core/sorting.py", line 436, in sort_mixed nums = np.sort(values[~str_pos]) File "<__array_function__ internals>", line 6, in sort File "/home/rstreet/miniconda3/envs/goodman_pipeline/lib/python3.6/site-packages/numpy/core/fromnumeric.py", line 989, in sort a.sort(axis=axis, kind=kind, order=order) TypeError: '<' not supported between instances of 'Undefined' and 'int'

simontorres commented 4 years ago

@rachel3834 could you please paste your pip --freeze output?

simontorres commented 6 months ago

Changed to low priority because it is not really required lately, there is an implementation but @Allonck said it is not working as expected and I would say that this implementation was not completed.