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

Cannot use compressed .fits.gz files for flats #8

Closed
followthesheep opened this issue Jan 9, 2015 · 8 comments
Closed

Cannot use compressed .fits.gz files for flats #8

followthesheep opened this issue Jan 9, 2015 · 8 comments

Comments

@followthesheep
Copy link
Contributor

It looks like if the data file is compressed in .fit.gz, the flat combine routine crashes. This is because underneath, it is using IRAF's imcombine in IO.py. The internal python modules that use pyfits seem to work fine with gz files.

It would be useful to support gz files because that is what we get out of the KOA archive.

@lucarizzi
Copy link

This is a decision that might require some thinking. Basically, the issue is a general one where we need to decide if we want to stick with pyraf or move to other resources such as astropy.
For example, astropy has a package called ccdproc, which contains a Combiner class. It does all that imcombine does and more.
My issue with astropy is that it changes rapidly but I am happy to consider the option.
Thoughts?

@followthesheep
Copy link
Contributor Author

I think that it would be good to switch to astropy for a couple of reasons: (1) it has absorbed a number of astronomy related projects like pyfits, which is now where all the updates are and will reduce the number of dependencies, (2) it is already included in ureka so most people will have it. It also has some nice utilities dealing with tables, etc.

The issue with ccdproc is that is not part of the main astropy library, which by now is more stable. ccdproc is an astropy affiliated package, which means by default is not installed with ureka. Its api is also subject to changes.

I think it would be good to move away from reliance on iraf, but I"m not sure how many routines will need to be replaced. From a cursory search, I see the following routines: geoxytran, imarith, imcombine

@csteidel
Copy link

Hi Luca,
Where can I find information about the astropy ccdproc package? I would like to see what
options there are for filtering/combining images if they are better than the iraf options.

   Thanks,
      Chuck

On Jan 8, 2015, at 5:18 PM, lucarizzi notifications@github.com wrote:

This is a decision that might require some thinking. Basically, the issue is a general one where we need to decide if we want to stick with pyraf or move to other resources such as astropy.
For example, astropy has a package called ccdproc, which contains a Combiner class. It does all that imcombine does and more.
My issue with astropy is that it changes rapidly but I am happy to consider the option.
Thoughts?


Reply to this email directly or view it on GitHub #8 (comment).

@followthesheep
Copy link
Contributor Author

The help page for ccdproc is here: http://ccdproc.readthedocs.org/en/latest/ccdproc/index.html#

@nickkonidaris
Copy link

One suggestion on the imcombine topic. Perhaps it's better to use the
version that stsci created and has tested. I'm going to start using the
stsci version for another pipeline I'm helping on. I'll report back if
there are problems, but my guess is there won't be.

The name is:
stsci.image.numcombine.numCombine

Docs:

http://ssb.stsci.edu/doc/stsci_python_x/stsci.tools.doc/html/analysis.html

~ Nick

On Thu, Jan 8, 2015 at 5:18 PM, lucarizzi notifications@github.com wrote:

This is a decision that might require some thinking. Basically, the issue
is a general one where we need to decide if we want to stick with pyraf or
move to other resources such as astropy.
For example, astropy has a package called ccdproc, which contains a
Combiner class. It does all that imcombine does and more.
My issue with astropy is that it changes rapidly but I am happy to
consider the option.
Thoughts?

Reply to this email directly or view it on GitHub.

+1 831 704 6425

@lucarizzi
Copy link

Thanks Nick

Looks very promising. Keep us posted !

On Jan 17, 2015, at 6:51 AM, Nick Konidaris notifications@github.com wrote:

One suggestion on the imcombine topic. Perhaps it's better to use the
version that stsci created and has tested. I'm going to start using the
stsci version for another pipeline I'm helping on. I'll report back if
there are problems, but my guess is there won't be.

The name is:
stsci.image.numcombine.numCombine

Docs:

http://ssb.stsci.edu/doc/stsci_python_x/stsci.tools.doc/html/analysis.html

~ Nick

On Thu, Jan 8, 2015 at 5:18 PM, lucarizzi notifications@github.com wrote:

This is a decision that might require some thinking. Basically, the issue
is a general one where we need to decide if we want to stick with pyraf or
move to other resources such as astropy.
For example, astropy has a package called ccdproc, which contains a
Combiner class. It does all that imcombine does and more.
My issue with astropy is that it changes rapidly but I am happy to
consider the option.
Thoughts?

Reply to this email directly or view it on GitHub.

+1 831 704 6425

Reply to this email directly or view it on GitHub.

@csteidel
Copy link

Hi Luca, Nick-
I looked at both the versions of python combine tasks and in the one case
it is completely untested (astropython) while the STScI task is considerably
less versatile than the one in iraf. The STScI will continue to support the
pyraf tasks since it is now the case that most of the pipelines for the HST nstruments
are built around them. So, I think there is little impetus to move away from
the pyraf task until there is one that is actually an improvement in capability.
There is quite a bit of room in the pyraf imcombine task used in Rectify.py
for improving the performace of the image combining/CR rejection and I
think that is where any effort should be focused.

Chuck

On Jan 18, 2015, at 11:15 AM, lucarizzi notifications@github.com wrote:

Thanks Nick

Looks very promising. Keep us posted !

On Jan 17, 2015, at 6:51 AM, Nick Konidaris notifications@github.com wrote:

One suggestion on the imcombine topic. Perhaps it's better to use the
version that stsci created and has tested. I'm going to start using the
stsci version for another pipeline I'm helping on. I'll report back if
there are problems, but my guess is there won't be.

The name is:
stsci.image.numcombine.numCombine

Docs:

http://ssb.stsci.edu/doc/stsci_python_x/stsci.tools.doc/html/analysis.html

~ Nick

On Thu, Jan 8, 2015 at 5:18 PM, lucarizzi notifications@github.com wrote:

This is a decision that might require some thinking. Basically, the issue
is a general one where we need to decide if we want to stick with pyraf or
move to other resources such as astropy.
For example, astropy has a package called ccdproc, which contains a
Combiner class. It does all that imcombine does and more.
My issue with astropy is that it changes rapidly but I am happy to
consider the option.
Thoughts?

Reply to this email directly or view it on GitHub.

+1 831 704 6425

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub #8 (comment).

@nickkonidaris
Copy link

Hi Chuck,

The only time pyraf's imcombine is used is in the Flat field step. In other
parts of the code base, the pipeline uses a version of imcombine that I
wrote (e.g., Wavelength.imcombine, Background.imcombine).

As I understood it, changing from pyraf to stsci imcombine has some benefit
for the Keck side of things. I see no problem in making such a change.

I don't recommend swapping out the Background.imcombine with an iraf
imcombine version. At least me and Dawn have tried improving CRR using
pyraf imcombine without much success. So I don't think imcombine will help
in the Background step. I've recommended tools like lacosmic and Chuck's
recommended qrzap to try to improve things, but I haven't had success with
the former or tried the latter. To be honest, I didn't do the most
systematic job here. As far as performance, my code operates faster than
pyraf or iraf for the median step, at least on two linux servers (pharos
and ramekin) and the mac laptop that I've benchmarked against. So, I don't
see an easy way to swap in iraf.imcombine and improve CRR or performance.

The way to move forward is (for someone) to play with the background
stacking step by hand. They can use either iraf, pyraf, or lacosmic.
Hopefully someone can tune up something by hand to produce a method that
works better than what is currently in Background.py. If we have a method
that works better, it's easy to add the new method back into the pipeline.

To be specific, there's a file called (e.g.) Offset_1.5.txt. The first step
in Background.py is to stack up all the files in Offset_1.5.txt into
Offset_1.5.txt.fits. The goal is to develop a new method to stack the
frames in the Offset_1.5.txt file. Compare the new method against what's in
Offset_1.5.txt.fits. If there's an improvement, we should put it into the
pipeline.

I hope this helps!
~ N

On Sun, Jan 18, 2015 at 12:26 PM, csteidel notifications@github.com wrote:

Hi Luca, Nick-
I looked at both the versions of python combine tasks and in the one case
it is completely untested (astropython) while the STScI task is
considerably
less versatile than the one in iraf. The STScI will continue to support
the
pyraf tasks since it is now the case that most of the pipelines for the
HST nstruments
are built around them. So, I think there is little impetus to move away
from
the pyraf task until there is one that is actually an improvement in
capability.
There is quite a bit of room in the pyraf imcombine task used in
Rectify.py
for improving the performace of the image combining/CR rejection and I
think that is where any effort should be focused.

Chuck

On Jan 18, 2015, at 11:15 AM, lucarizzi notifications@github.com
wrote:

Thanks Nick

Looks very promising. Keep us posted !

On Jan 17, 2015, at 6:51 AM, Nick Konidaris notifications@github.com
wrote:

One suggestion on the imcombine topic. Perhaps it's better to use the
version that stsci created and has tested. I'm going to start using
the
stsci version for another pipeline I'm helping on. I'll report back if
there are problems, but my guess is there won't be.

The name is:
stsci.image.numcombine.numCombine

Docs:

http://ssb.stsci.edu/doc/stsci_python_x/stsci.tools.doc/html/analysis.html

~ Nick

On Thu, Jan 8, 2015 at 5:18 PM, lucarizzi notifications@github.com
wrote:

This is a decision that might require some thinking. Basically, the
issue
is a general one where we need to decide if we want to stick with
pyraf or
move to other resources such as astropy.
For example, astropy has a package called ccdproc, which contains a
Combiner class. It does all that imcombine does and more.
My issue with astropy is that it changes rapidly but I am happy to
consider the option.
Thoughts?

Reply to this email directly or view it on GitHub.

+1 831 704 6425

Reply to this email directly or view it on GitHub.

Reply to this email directly or view it on GitHub <
https://github.com/Mosfire-DataReductionPipeline/MosfireDRP/issues/8#issuecomment-70421159>.

Reply to this email directly or view it on GitHub
#8 (comment)
.

+1 831 704 6425

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

No branches or pull requests

5 participants