search

cxcsds / ciao-contrib

Extra scripts and code to enhance the capabilities of CIAO.
GNU General Public License v3.0
8 stars 6 forks source link

reproject_obs incompatibility with Python 3 #10

Closed nplee closed 7 years ago

nplee commented 7 years ago

from helpdesk;

reproject_obs still uses "xrange" instead of "range", which will fail in Python 3.

Changing lines 371-372 from:

                newnames = []
                for (idx, afile) in zip(xrange(0, nfiles), afiles):

to:

                newnames = []
                try: xrange(0, nfiles)
                except NameError: xrange = range
                for (idx, afile) in zip(xrange(0, nfiles), afiles):

seems to fix the issue.

DougBurke commented 7 years ago

There's obviously a way to trigger this (hence the helpdesk ticket); do we know what it is? If so, we can perhaps add it as a regression test.

We don't need the try/except block; we can 'import six' and then use six.range (or whatever it's called).

On Fri, Feb 24, 2017 at 11:21 AM nplee notifications@github.com wrote:

from helpdesk;

reproject_obs still uses "xrange" instead of "range", which will fail in Python 3.

Changing lines 371-372 from:

            newnames = []
            for (idx, afile) in zip(xrange(0, nfiles), afiles):

to:

            newnames = []
            try: xrange(0, nfiles)
            except NameError: xrange = range
            for (idx, afile) in zip(xrange(0, nfiles), afiles):

seems to fix the issue.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/cxcsds/ciao-contrib/issues/10, or mute the thread https://github.com/notifications/unsubscribe-auth/AANtftLN--OEdriwwzCKct-HZKzSxP7nks5rfwN7gaJpZM4MLZDo .

nplee commented 7 years ago

It seems like any arbitrary call to reproject_obs triggers this failure (I haven't looked too deep into it yet, but see the two examples below):

falafel-250: reproject_obs @evt.lis test/merged merge=yes clobber+
Running reproject_obs
Version: 27 October 2016

Verifying 4 observations.
Calculating new tangent point.
New tangent point: RA=7h 17m 29.279999999999546s Dec=37d 46' 11.06399999999553"

Observations to be reprojected:

  Obsid  Obs Date   Exp    DETNAM     SIM_Z    FP   Sepn   PA  
                   (ks)                (mm)    (K)   (')  (deg)
---------------------------------------------------------------
1 1655  2001-01-29  19.9 ACIS-01236  -225.843 153.3   3.5  -149
2 4200  2003-01-08  59.0 ACIS-01236  -241.794 154.1   3.0   +97
3 16305 2013-12-11  94.3 ACIS-0123   -225.843 156.0   2.6   -20
4 16235 2013-12-13  70.2 ACIS-0123   -225.843 155.5   1.0   -17

WARNING - DATAMODE values differ:
  Obsids 1655 16235 have DATAMODE=FAINT and the rest have VFAINT

# reproject_obs (27 October 2016): ERROR name 'xrange' is not defined

falafel-253: reproject_obs "1655/repro/acisf01655_repro_evt2.fits,4200/repro/acisf04200_repro_evt2.fits" test/merged merge=no clobber+
Running reproject_obs
Version: 27 October 2016

Verifying 2 observations.
Calculating new tangent point.
New tangent point: RA=7h 17m 32.2319999999997s Dec=37d 44' 30.84000000000401"

Observations to be reprojected:

  Obsid  Obs Date   Exp    DETNAM     SIM_Z    FP   Sepn   PA  
                   (ks)                (mm)    (K)   (')  (deg)
---------------------------------------------------------------
1 1655  2001-01-29  19.9 ACIS-01236  -225.843 153.3   2.7  -119
2 4200  2003-01-08  59.0 ACIS-01236  -241.794 154.1   2.7   +61

WARNING - DATAMODE values differ:
  Obsid 1655 has DATAMODE=FAINT and 4200 has VFAINT

# reproject_obs (27 October 2016): ERROR name 'xrange' is not defined

but for some reason, using the same stack or string of files with merge_obs, which runs reproject_obs, the error isn't triggered.

falafel-259: merge_obs "1655/repro/acisf01655_repro_evt2.fits,4200/repro/acisf04200_repro_evt2.fits" outroot=test clobber+
Running merge_obs
Version: 12 September 2016

Verifying 2 observations.
Using CSC ACIS broad science energy band.
Calculating new tangent point.
New tangent point: RA=7h 17m 32.2319999999997s Dec=37d 44' 30.84000000000401"

Observations to be reprojected:

  Obsid  Obs Date   Exp    DETNAM     SIM_Z    FP   Sepn   PA  
                   (ks)                (mm)    (K)   (')  (deg)
---------------------------------------------------------------
1 1655  2001-01-29  19.9 ACIS-01236  -225.843 153.3   2.7  -119
2 4200  2003-01-08  59.0 ACIS-01236  -241.794 154.1   2.7   +61

WARNING - DATAMODE values differ:
  Obsid 1655 has DATAMODE=FAINT and 4200 has VFAINT

Running tasks in parallel with 4 processors.
Reprojecting 2 event files to a common tangent point.
Merging reprojected events files to test_merged_evt.fits

Calculating the output grid

The merged images will have 439 by 520 pixels, a pixel size of 3.936 arcsec,
    and cover x=2672.5:6184.5:8, y=1464.5:5624.5:8.

Creating the fluxed images for 2 observations in parallel.
Creating aspect histograms for obsid 4200
Creating aspect histograms for obsid 1655
Creating 5 instrument maps for obsid 1655
Creating 5 instrument maps for obsid 4200
Creating 5 exposure maps for obsid 1655
Combining 5 exposure maps for obsid 1655
Thresholding data for obsid 1655
Exposure-correcting image for obsid 1655
Creating 5 exposure maps for obsid 4200
Combining 5 exposure maps for obsid 4200
Thresholding data for obsid 4200
Exposure-correcting image for obsid 4200

Combining 2 observations.

The following files were created:

The co-added clipped counts image is:
     test_broad_thresh.img

The co-added clipped exposure map is:
     test_broad_thresh.expmap

The co-added exposure-corrected image is:
     test_broad_flux.img

Warning: the merged event file test_merged_evt.fits
   should not be used to create ARF/RMF/exposure maps because
      the RA_NOM keyword varies by 0.10041663826000047 (limit is 0.0003)
      the DEC_NOM keyword varies by 0.04412834210900485 (limit is 0.0003)
      the ROLL_NOM keyword varies by 46.13544245199 (limit is 1.0)
      the SIM_Z keyword varies by 15.950816273100003 (limit is 0.1)
      the aim points fall on CCDs: 0 3
        which means that the ONTIME/LIVETIME/EXPOSURE keywords
        do not reflect the full observation length.
DougBurke commented 7 years ago

So, it looks like we should add at least one test that compares merge_obs to reproject_obs followed by flux_obs (since they are meant to do the same thing).

DougBurke commented 7 years ago

So, the error happens when you use an obsid which has multiple asol files - e.g. 4200 which has

% find . -name \*asol\*
./4200/primary/pcadf158389428N004_asol1.fits.gz
./4200/primary/pcadf158377409N004_asol1.fits.gz
./1655/primary/pcadf097142619N003_asol1.fits.gz

This is why I hadn't seen it in my testing; I just happened to not use such an obsid...

DougBurke commented 7 years ago

Fix by #30