No more Ureka!

[lucarizzi]

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

[dmkassis]

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi notifications@github.com<mailto:notifications@github.com> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52

[followthesheep]

Which version of MOSFIRE works with Anaconda at the moment?

Really the reason we are dependent on ureka is iraf. Should we accelerate our plans for testing and removing iraf dependency? We may want to use a testing framework so that we can run tests faster and in a consistent way. When I packaged MOSFIRE as a python package, I left in the ability to do tests with pytest

On Tue, May 24, 2016, 3:36 PM Marc Kassis notifications@github.com wrote:

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi <notifications@github.com<mailto: notifications@github.com>> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub< https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52>

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221179152

[lucarizzi]

The branch called imcombine works with anaconda and is independent from Iraf. To make sure I am running it on a computer that does not have Iraf installed You have to manually install ccdproc

I am testing it right now. In the short term we need a test suite. In the long term we need to refactor the entire code so that we can do unit tests

On May 23, 2016, at 8:52 PM, Tuan Do notifications@github.com wrote:

Which version of MOSFIRE works with Anaconda at the moment?

Really the reason we are dependent on ureka is iraf. Should we accelerate our plans for testing and removing iraf dependency? We may want to use a testing framework so that we can run tests faster and in a consistent way. When I packaged MOSFIRE as a python package, I left in the ability to do tests with pytest

On Tue, May 24, 2016, 3:36 PM Marc Kassis notifications@github.com wrote:

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi <notifications@github.com<mailto: notifications@github.com>> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub< https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52>

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221179152

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

[followthesheep]

Should we start a new issue to discuss the tests we want to put in the near term? I would feel better about abandoning IRAF if we had some tests that everyone can run.

On Tue, May 24, 2016 at 3:58 PM lucarizzi notifications@github.com wrote:

The branch called imcombine works with anaconda and is independent from Iraf. To make sure I am running it on a computer that does not have Iraf installed You have to manually install ccdproc

I am testing it right now. In the short term we need a test suite. In the long term we need to refactor the entire code so that we can do unit tests

On May 23, 2016, at 8:52 PM, Tuan Do notifications@github.com wrote:

Which version of MOSFIRE works with Anaconda at the moment?

Really the reason we are dependent on ureka is iraf. Should we accelerate our plans for testing and removing iraf dependency? We may want to use a testing framework so that we can run tests faster and in a consistent way. When I packaged MOSFIRE as a python package, I left in the ability to do tests with pytest

On Tue, May 24, 2016, 3:36 PM Marc Kassis notifications@github.com wrote:

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi <notifications@github.com <mailto: notifications@github.com>> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub< https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52>

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221179152

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221182731

[lucarizzi]

Sure that is a good idea.

I should mention that Iraf was only used for a rather simple combine of images and in only one case.

If we coming a set of images with the old algorithm and do it again with the new one and they are the same we should be fine

Iraf was also used to convert mm to pixels using geoxytran. I have rewritten that too a while ago and I tested the conversion back and forth a number of times: the results are identical within numerical precision.

On May 23, 2016, at 9:02 PM, Tuan Do notifications@github.com wrote:

Should we start a new issue to discuss the tests we want to put in the near term? I would feel better about abandoning IRAF if we had some tests that everyone can run.

On Tue, May 24, 2016 at 3:58 PM lucarizzi notifications@github.com wrote:

The branch called imcombine works with anaconda and is independent from Iraf. To make sure I am running it on a computer that does not have Iraf installed You have to manually install ccdproc

I am testing it right now. In the short term we need a test suite. In the long term we need to refactor the entire code so that we can do unit tests

On May 23, 2016, at 8:52 PM, Tuan Do notifications@github.com wrote:

Which version of MOSFIRE works with Anaconda at the moment?

Really the reason we are dependent on ureka is iraf. Should we accelerate our plans for testing and removing iraf dependency? We may want to use a testing framework so that we can run tests faster and in a consistent way. When I packaged MOSFIRE as a python package, I left in the ability to do tests with pytest

On Tue, May 24, 2016, 3:36 PM Marc Kassis notifications@github.com wrote:

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi <notifications@github.com <mailto: notifications@github.com>> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub< https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52>

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221179152

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221182731

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

[joshwalawender]

FYI, The new imcombine branch does not currently handle the different combine types such as sigma clip and minimax rejection. If I recall correctly, some of the rejection methods took slightly different arguments in IRAF combine compared to ccdproc.combine. I wasn't sure which of those rejection methods were actually used by the code, so I just punted on passing those arguments while we decided how to proceed. We should figure out what we need to support and if we have to maintain the old argument formats or just accept the new ccdproc.combine arguments and support those from now on.

[followthesheep]

We should probably support both sigma clipping and minmax rejection, since those are the most common. I think as along as we have tests for both of these methods for ccdproc, then it should be okay to use the ccdproc implementation. The thing I'm worried about is that ccdproc is prone to changing their api. I suppose we just need to keep up if we go this direction.

On Tue, May 24, 2016 at 4:38 PM Josh Walawender notifications@github.com wrote:

FYI, The new imcombine branch does not currently handle the different combine types such as sigma clip and minimax rejection. If I recall correctly, some of the rejection methods took slightly different arguments in IRAF combine compared to ccdproc.combine. I wasn't sure which of those rejection methods were actually used by the code, so I just punted on passing those arguments while we decided how to proceed. We should figure out what we need to support and if we have to maintain the old argument formats or just accept the new ccdproc.combine arguments and support those from now on.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221189898

[lucarizzi]

Tuan We don't really have a choice. There seem to be some fundamental incompatibility with Iraf and some late versions of Linux and glibc.

But also remember that imcombine is only used once and only with one rejection algorithm that is currently hard coded. I believe it is at the level of combining either the flats or the wavelength files.

The actual scientific frames combination does not use Iraf but combines the 2d arrays directly.

If we have to, we can just create a locks copy of that particular task,ship it with the pipeline and forget about ccdproc altogether. There are many ways ;-)

On May 23, 2016, at 9:55 PM, Tuan Do notifications@github.com wrote:

We should probably support both sigma clipping and minmax rejection, since those are the most common. I think as along as we have tests for both of these methods for ccdproc, then it should be okay to use the ccdproc implementation. The thing I'm worried about is that ccdproc is prone to changing their api. I suppose we just need to keep up if we go this direction.

On Tue, May 24, 2016 at 4:38 PM Josh Walawender notifications@github.com wrote:

FYI, The new imcombine branch does not currently handle the different combine types such as sigma clip and minimax rejection. If I recall correctly, some of the rejection methods took slightly different arguments in IRAF combine compared to ccdproc.combine. I wasn't sure which of those rejection methods were actually used by the code, so I just punted on passing those arguments while we decided how to proceed. We should figure out what we need to support and if we have to maintain the old argument formats or just accept the new ccdproc.combine arguments and support those from now on.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221189898

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

[followthesheep]

Do you mean that we have no choice but to use ccdproc or that ccdproc has no choices?

One reason that we should pay attention to the combination is that the default doesn't work well for bright stars when getting the sky lines for wavelength calibration. I've been working on a branch to try to optimize to work out this issue. I think we can use ccdproc, but I'm just advocating that we don't hard code the method, but allow for the available rejection methods.

On Tue, May 24, 2016 at 5:18 PM lucarizzi notifications@github.com wrote:

Tuan We don't really have a choice. There seem to be some fundamental incompatibility with Iraf and some late versions of Linux and glibc.

But also remember that imcombine is only used once and only with one rejection algorithm that is currently hard coded. I believe it is at the level of combining either the flats or the wavelength files.

The actual scientific frames combination does not use Iraf but combines the 2d arrays directly.

If we have to, we can just create a locks copy of that particular task,ship it with the pipeline and forget about ccdproc altogether. There are many ways ;-)

On May 23, 2016, at 9:55 PM, Tuan Do notifications@github.com wrote:

We should probably support both sigma clipping and minmax rejection, since those are the most common. I think as along as we have tests for both of these methods for ccdproc, then it should be okay to use the ccdproc implementation. The thing I'm worried about is that ccdproc is prone to changing their api. I suppose we just need to keep up if we go this direction.

On Tue, May 24, 2016 at 4:38 PM Josh Walawender < notifications@github.com> wrote:

FYI, The new imcombine branch does not currently handle the different combine types such as sigma clip and minimax rejection. If I recall correctly, some of the rejection methods took slightly different arguments in IRAF combine compared to ccdproc.combine. I wasn't sure which of those rejection methods were actually used by the code, so I just punted on passing those arguments while we decided how to proceed. We should figure out what we need to support and if we have to maintain the old argument formats or just accept the new ccdproc.combine arguments and support those from now on.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221189898

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221198382

[lucarizzi]

On May 23, 2016, at 10:31 PM, Tuan Do notifications@github.com wrote:

Do you mean that we have no choice but to use ccdproc or that ccdproc has no choices?

I mean that we might no choice but to abandon Iraf. What we do about it is open for discussion. It's a simple combination of arrays: even numpy can take care of it if we spend enough time implementing the rejection algorithms. Maybe the best approach is to see which methods are available on ccdproc and contribute to their code to implement the missing ones. We would help the community !

One reason that we should pay attention to the combination is that the default doesn't work well for bright stars when getting the sky lines for wavelength calibration. I've been working on a branch to try to optimize to work out this issue. I think we can use ccdproc, but I'm just advocating that we don't hard code the method, but allow for the available rejection methods.

I agree but this is a modification of the pipeline. A welcome one for sure and probably quite easy.

On Tue, May 24, 2016 at 5:18 PM lucarizzi notifications@github.com wrote:

Tuan We don't really have a choice. There seem to be some fundamental incompatibility with Iraf and some late versions of Linux and glibc.

But also remember that imcombine is only used once and only with one rejection algorithm that is currently hard coded. I believe it is at the level of combining either the flats or the wavelength files.

The actual scientific frames combination does not use Iraf but combines the 2d arrays directly.

If we have to, we can just create a locks copy of that particular task,ship it with the pipeline and forget about ccdproc altogether. There are many ways ;-)

On May 23, 2016, at 9:55 PM, Tuan Do notifications@github.com wrote:

We should probably support both sigma clipping and minmax rejection, since those are the most common. I think as along as we have tests for both of these methods for ccdproc, then it should be okay to use the ccdproc implementation. The thing I'm worried about is that ccdproc is prone to changing their api. I suppose we just need to keep up if we go this direction.

On Tue, May 24, 2016 at 4:38 PM Josh Walawender < notifications@github.com> wrote:

FYI, The new imcombine branch does not currently handle the different combine types such as sigma clip and minimax rejection. If I recall correctly, some of the rejection methods took slightly different arguments in IRAF combine compared to ccdproc.combine. I wasn't sure which of those rejection methods were actually used by the code, so I just punted on passing those arguments while we decided how to proceed. We should figure out what we need to support and if we have to maintain the old argument formats or just accept the new ccdproc.combine arguments and support those from now on.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221189898

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221198382

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

[lucarizzi]

So ccdproc implements min max and sigma clipping already.
Interestingly it also implements la cosmic for cosmic ray rejection.

Definitely worth considering.

On May 23, 2016, at 10:31 PM, Tuan Do notifications@github.com wrote:

Do you mean that we have no choice but to use ccdproc or that ccdproc has no choices?

One reason that we should pay attention to the combination is that the default doesn't work well for bright stars when getting the sky lines for wavelength calibration. I've been working on a branch to try to optimize to work out this issue. I think we can use ccdproc, but I'm just advocating that we don't hard code the method, but allow for the available rejection methods.

On Tue, May 24, 2016 at 5:18 PM lucarizzi notifications@github.com wrote:

Tuan We don't really have a choice. There seem to be some fundamental incompatibility with Iraf and some late versions of Linux and glibc.

But also remember that imcombine is only used once and only with one rejection algorithm that is currently hard coded. I believe it is at the level of combining either the flats or the wavelength files.

The actual scientific frames combination does not use Iraf but combines the 2d arrays directly.

If we have to, we can just create a locks copy of that particular task,ship it with the pipeline and forget about ccdproc altogether. There are many ways ;-)

On May 23, 2016, at 9:55 PM, Tuan Do notifications@github.com wrote:

We should probably support both sigma clipping and minmax rejection, since those are the most common. I think as along as we have tests for both of these methods for ccdproc, then it should be okay to use the ccdproc implementation. The thing I'm worried about is that ccdproc is prone to changing their api. I suppose we just need to keep up if we go this direction.

On Tue, May 24, 2016 at 4:38 PM Josh Walawender < notifications@github.com> wrote:

FYI, The new imcombine branch does not currently handle the different combine types such as sigma clip and minimax rejection. If I recall correctly, some of the rejection methods took slightly different arguments in IRAF combine compared to ccdproc.combine. I wasn't sure which of those rejection methods were actually used by the code, so I just punted on passing those arguments while we decided how to proceed. We should figure out what we need to support and if we have to maintain the old argument formats or just accept the new ccdproc.combine arguments and support those from now on.

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221189898

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221198382

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

[csteidel]

Hi everyone, Perhaps this might be a good opportunity to improve (what I consider to be) the least-optimal section of the DRP at present— the combining of frames/CR rejection section, where things are currently hard-wired depending on how many frames are going into each stack, and as currently coded are extremely wasteful of good data in order to clean CRs from the A or B stacks. I like the idea of including some kind of morphological rejection of CRs (such as lacosmic) for individual frames so that brute force algorithms like minmax reject are never necessary. Even the sigreject implementation will only work if variations in the sky (or objects in the case of bright targets) are much smaller than the CR intensity (not always true) . I think the results would be markedly improved if CRs were identified using a temporary background subtraction (e.g., subtraction of temporally local disregistered frames), morphological CR rejection that adds pixels to a mask that applies to that frame only, and then the stacking would be done by applying the masks.

For bright objects, I suspect that a 1d or 2d running median spatial profile could be used to identify outlier pixels; it may be that for high S/N regimes with a small number of frames, bad pixels or those hit by CRs should be replaced (i.e. interpolated rather than masked or removed) using the running profile (this is implemented in IRAF’s apextract, for example).

BTW, so far I have been very unimpressed with the stability of all the various python distributions! Can we find one that will be around for more than a year before being abandoned? At some point I have looked at some of the python packages that do image processing of various kinds and they seemed highly incomplete (in terms of algorithms supported) and/or untested- I suggest we either write our own from scratch or rely only on modules that have been stable for a long time…

On May 24, 2016, at 12:14 AM, lucarizzi notifications@github.com<mailto:notifications@github.com> wrote:

Sure that is a good idea.

I should mention that Iraf was only used for a rather simple combine of images and in only one case.

If we coming a set of images with the old algorithm and do it again with the new one and they are the same we should be fine

Iraf was also used to convert mm to pixels using geoxytran. I have rewritten that too a while ago and I tested the conversion back and forth a number of times: the results are identical within numerical precision.

On May 23, 2016, at 9:02 PM, Tuan Do notifications@github.com<mailto:notifications@github.com> wrote:

Should we start a new issue to discuss the tests we want to put in the near term? I would feel better about abandoning IRAF if we had some tests that everyone can run.

On Tue, May 24, 2016 at 3:58 PM lucarizzi notifications@github.com<mailto:notifications@github.com> wrote:

The branch called imcombine works with anaconda and is independent from Iraf. To make sure I am running it on a computer that does not have Iraf installed You have to manually install ccdproc

I am testing it right now. In the short term we need a test suite. In the long term we need to refactor the entire code so that we can do unit tests

On May 23, 2016, at 8:52 PM, Tuan Do notifications@github.com<mailto:notifications@github.com> wrote:

Which version of MOSFIRE works with Anaconda at the moment?

Really the reason we are dependent on ureka is iraf. Should we accelerate our plans for testing and removing iraf dependency? We may want to use a testing framework so that we can run tests faster and in a consistent way. When I packaged MOSFIRE as a python package, I left in the ability to do tests with pytest

On Tue, May 24, 2016, 3:36 PM Marc Kassis notifications@github.com<mailto:notifications@github.com> wrote:

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi notifications@github.com<mailto:notifications@github.com <mailto: notifications@github.commailto:notifications@github.com>> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub< https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52>

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221179152

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221182731

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221185423

[lucarizzi]

Yes I agree completely.

On the issue of the different distributions, he good news is that it doesn't really affect us. The dependency on Iraf was a big deal but once we get rid of it, our dependencies are on packages and not on distributions.

I believe we can narrow it down to having pyfits (either standalone, deprecated, or from astropy.io), scipy and numpy. Might be others. But the point is that any Python distribution which provides those packages or where they can be installed will work.

So from now we won't be telling people how to run their Python and which distribution to use but just which packages they need to install to run the pipeline. And they will be so basic that most users will have them already.

In fact, a few months back, I was able to run the old version of the pipeline on anaconda without changing a line of code but manually installing pyraf.

On May 24, 2016, at 5:41 AM, csteidel notifications@github.com wrote:

Hi everyone, Perhaps this might be a good opportunity to improve (what I consider to be) the least-optimal section of the DRP at present— the combining of frames/CR rejection section, where things are currently hard-wired depending on how many frames are going into each stack, and as currently coded are extremely wasteful of good data in order to clean CRs from the A or B stacks. I like the idea of including some kind of morphological rejection of CRs (such as lacosmic) for individual frames so that brute force algorithms like minmax reject are never necessary. Even the sigreject implementation will only work if variations in the sky (or objects in the case of bright targets) are much smaller than the CR intensity (not always true) . I think the results would be markedly improved if CRs were identified using a temporary background subtraction (e.g., subtraction of temporally local disregistered frames), morphological CR rejection that adds pixels to a mask that applies to that frame only, and then the stacking would be done by applying the masks.

For bright objects, I suspect that a 1d or 2d running median spatial profile could be used to identify outlier pixels; it may be that for high S/N regimes with a small number of frames, bad pixels or those hit by CRs should be replaced (i.e. interpolated rather than masked or removed) using the running profile (this is implemented in IRAF’s apextract, for example).

BTW, so far I have been very unimpressed with the stability of all the various python distributions! Can we find one that will be around for more than a year before being abandoned? At some point I have looked at some of the python packages that do image processing of various kinds and they seemed highly incomplete (in terms of algorithms supported) and/or untested- I suggest we either write our own from scratch or rely only on modules that have been stable for a long time…

On May 24, 2016, at 12:14 AM, lucarizzi notifications@github.com<mailto:notifications@github.com> wrote:

Sure that is a good idea.

I should mention that Iraf was only used for a rather simple combine of images and in only one case.

If we coming a set of images with the old algorithm and do it again with the new one and they are the same we should be fine

Iraf was also used to convert mm to pixels using geoxytran. I have rewritten that too a while ago and I tested the conversion back and forth a number of times: the results are identical within numerical precision.

On May 23, 2016, at 9:02 PM, Tuan Do notifications@github.com<mailto:notifications@github.com> wrote:

Should we start a new issue to discuss the tests we want to put in the near term? I would feel better about abandoning IRAF if we had some tests that everyone can run.

On Tue, May 24, 2016 at 3:58 PM lucarizzi notifications@github.com<mailto:notifications@github.com> wrote:

The branch called imcombine works with anaconda and is independent from Iraf. To make sure I am running it on a computer that does not have Iraf installed You have to manually install ccdproc

I am testing it right now. In the short term we need a test suite. In the long term we need to refactor the entire code so that we can do unit tests

On May 23, 2016, at 8:52 PM, Tuan Do notifications@github.com<mailto:notifications@github.com> wrote:

Which version of MOSFIRE works with Anaconda at the moment?

Really the reason we are dependent on ureka is iraf. Should we accelerate our plans for testing and removing iraf dependency? We may want to use a testing framework so that we can run tests faster and in a consistent way. When I packaged MOSFIRE as a python package, I left in the ability to do tests with pytest

On Tue, May 24, 2016, 3:36 PM Marc Kassis notifications@github.com<mailto:notifications@github.com> wrote:

I support this decision. And surprised that ureka was so quickly abandoned.

Marc

On May 23, 2016, at 7:58 PM, lucarizzi notifications@github.com<mailto:notifications@github.com <mailto: notifications@github.commailto:notifications@github.com>> wrote:

While I was testing the compatibility of the new pipeline version with the old Ureka, I realized that Ureka is now deprecated and unsupported. It is also apparently broken on the newest versio of Linux. The Ureka team has moved to Anaconda, producing they own package as an add - on.

I then suggest that we stop making sure that the pipeline works with Ureka and just suggest that our new adopted version is Anaconda. I might want to check if Entought works. Everybody ok with this?

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub< https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52>

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub < https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221179152

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you commented.

Reply to this email directly or view it on GitHub https://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221182731

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHubhttps://github.com/Keck-DataReductionPipelines/MosfireDRP/issues/52#issuecomment-221185423

— You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub

[joshwalawender]

I'm closing this issue as we approach release of the new 2016 version because that version of the DRP works without Ureka/IRAF. Some of the topics raised above (such as CR rejection) are not resolved in that release, but can be raised as separate issues/feature requests.