Closed stscijgbot closed 4 years ago
Comment by Alicia Canipe: Added to the list of issues for prioritization: [https://outerspace.stsci.edu/display/JWSTPWG/Calibration+Pipeline+Deadlines+and+INS+Issues]
Comment by David Law: From an email thread: Rather than trying to do any actual cube construction in a distorted reference frame (requiring mapping coordinates from one image to another), the simplest way to implement it is as a rotation in the xi-eta cube coordinate frame with respect to RA/DEC. How cube building works is that each pixel on the detector (i.e., all slices) maps to a ‘point’ in RA/DEC space using the instrument WCS, and there is only a single step of gridding that occurs when building the cube in this output space from the input point cloud. By default, N is up and E left in the output cubes. What we planned to do for MIRI was to implement a very simple variation on the above in which the point cloud coordinates are simply rotated by some angle prior to constructing the xi-eta pixel coordinates used for the actual cube construction. This angle is determined from the WCS as the angle for which the first exposure in a given sequence will have the along-slice direction in the cube-x direction and the across-slice in the cube-y direction. The embedded WCS in these derotated cubes is fully useful as it’s simply a version of RA/DEC oriented without N up. The main intent of this coordinate frame is for PSF determination during commissioning, since for MIRI the PSF has a dependence on along-slice vs across-slice direction, but I’d expect it would be useful for science purposes too.
Comment by David Law: Here's a pipeline branch in which I calculated the relevant rotation angle and added it to the output cube headers: https://github.com/drlaw1558/jwst/tree/drl_abrot2 (this draft specific to MIRI though)
Comment by Jane Morrison: [~dlaw] Ok David I am going to start on this tomorrow. Do you have any test data I can use. We just want to dithered MRS data right ?
Comment by David Law: Any dithered MRS data should be fine, e.g. the stuff that I emailed you at the end of February from end-to-end testing should be good.
Comment by Jane Morrison: [~dlaw] So currently cube build can create an IFU cube in alpha-beta space, but it can only do it for single exposures not dithered. This is controlled by coord_system parameter = 'alpha-beta' These cubes are then built in alpha beta space and not using the point cloud because the transformation goes from x,y to local MRS coordinate system. For now I will just keep that and not bother taking it out. I can run a test to make sure the new de-rotated cubes are similar to the alpha_beta cubes using the old code.
But for de-rotated cubes these are only valid for a single band and single channel. I will add some checks in code to test this. I looked at your code and it seems simple enough. I suppose we want these cubes to have some distinctive name (suffix or something) - ideas ?
Comment by David Law: I think it's reasonable to keep in the old one-exposure alpha-beta cubes.
For the new cubes, I'm not sure that it's strictly necessary that they be only valid for a single band/channel. I'd be inclined to allow them for any combination of input exposures ( using the first one to set the rotation angle) with the simple documentation caveat that there could be a degree or so difference if (say) you're effectively using a 1A input file to set the rotation used for a 2A input as well. The difference in rotation angle between bands especially is sufficiently small that for many purposes it can likely be ignored when working with derotated cubes.
As far as a naming scheme, I think 'derotated' is fine. Alternatively, we could approach this the other way and instead of setting a special flag to create derotated cubes, we have a special flag that creates our usual aligned with N-E cubes. I.e., by default cube building runs with the 'SKYALIGN' flag enabled, which makes it produce cubes in which N is up and E left. If that flag is false, it produces what we've been calling derotated cubes. Thoughts? 'skyalign' is perhaps more descriptive than 'derotated'.
Comment by David Law: Just realized you might have been asked a slightly different question regarding file naming conventions; I'm inclined to not give these a special name. It's not really necessary, as the embedded WCS is fully accurate, and it's simply a construction choice similar to the weighting function used. I think it would just add complexity that we don't need.
Comment by Jane Morrison: You know better what is needed for commissioning than I do. But in the case of the alpha-beta type cubes there is 1 to 1 mapping of the slices to the beta dimension. So all alpha-beta cubes in channel 1 have a dimension of 21 in the beta dimension. We only had to worry about mapping and overlaps in the alpha and wavelength dimension. This ensured that the cubes were aligned in the alpha-beta system. I am not really sure what de-rotated cubes using more than 1 channels worth of data worth of data will be used for. Are you sure we want to allow the user to make these ? They seem a little confusing - but like I said you know better than me what they want these in commissioning for.
And back to SKYALIGN - yes I like that idea. That will be an additional header keyword that will be written to all IFU cubes. False if it is 'de-rotated". Have you talked to NIRSPEC about if they want something similar ? James Muzzerole may want these types of cube when they have internal lamp data.
Comment by Jane Morrison: [~dlaw]You know better what is needed for commissioning than I do. But in the case of the alpha-beta type cubes there is 1 to 1 mapping of the slices to the beta dimension. So all alpha-beta cubes in channel 1 have a dimension of 21 in the beta dimension. We only had to worry about mapping and overlaps in the alpha and wavelength dimension. This ensured that the cubes were aligned in the alpha-beta system. I am not really sure what de-rotated cubes using more than 1 channels worth of data worth of data will be used for. Are you sure we want to allow the user to make these ? They seem a little confusing - but like I said you know better than me what they want these in commissioning for.
And back to SKYALIGN - yes I like that idea. That will be an additional header keyword that will be written to all IFU cubes. False if it is 'de-rotated". Have you talked to NIRSPEC about if they want something similar ? James Muzzerole may want these types of cube when they have internal lamp data.
Comment by Jane Morrison: [~dlaw] I have looked at the code in https://github.com/drlaw1558/jwst/tree/drl_abrot2 That calculates the angle to rotate the IFU. I might be missing something but I thought we wanted to build the IFU cube on this rotated system and not just build the cube on the sky and supply a rotation in in the wcs. Would it not be better to build the ifu cube on the rotated system. That way we have a better 1 to 1 mapping of columns of the IFU cube with beta slices. Isn't the point of building cube in the alpha-beta system so that an output spaxel only have 1 slice contributing to it.
Comment by David Law: [~morrison] I'll bounce the SKYALIGN suggestion off James and Nora.
Regarding multi-band cubes: the thought is mainly one of simplicity, if our 'de-rotated' cubes are produced just by having them aligned with the orientation of the first exposure in the sequence (and have fully accurate WCS) there is no a-priori reason to add all of the logic necessary to prohibit creating them in certain cases. It's true that given the small rotation between channels a Ch1+2 derotated cube might not be quite as useful for defining the PSF as separate Ch1 and Ch2 cubes, but there may still be some convenience to be had in a combined cube in that you can then measure what the PSF is doing pretty well at all wavelengths without needing to create, align, and extract from multiple different cubes.
The code at [https://github.com/drlaw1558/jwst/tree/drl_abrot2] was just an example of computing the rotation and saving the value somehow, it didn't actually apply it like we want to do. I've got a newer version though at [https://github.com/drlaw1558/jwst/tree/drl_abrot3] that does apply the rotation in a horribly-kludgy and approximate way.
About a similar capability for NIRSpec - while I don't speak in any functional capacity for NIRSpec, speaking as a science user of NIRSpec this is very much a capability that's desired, and will be strongly beneficial for a subset of science programs (including GTO and ERS programs as well as planned GO programs). Detailed PSF calibrations, and the ability to reconstruct cubes in an instrument frame that's independent of sky orientation, will be a key enabler for high contrast PSF subtractions (e.g. spectroscopy of substellar companions < 1 arcsec from host stars).
Comment by Jane Morrison: [~dlaw] in the equation ab2radec_transform = input_model.meta.wcs.get_transform('alpha_beta','world') temp_ra1, tempdec1, = ab2radec_transform(0,0,wave[0]) temp_ra2, tempdec2, = ab2radec_transform(0,-1,wave[0])
alpha = 0, beta = 0 and first wavelength value can you remind me what an alpha =0 and beta = 0 or -1 means in alpha-beta space
Comment by Jane Morrison: !Screen Shot 2020-05-20 at 8.33.17 AM.png|thumbnail! Is this diagram still valid. So an alpha of 0 is the origin - what is beta = -1
Comment by David Law: [~morrison] In that equation I've just taken one point at the center of the field of view (alpha=beta=0) and a second point offset by some small distance in beta (1 arcsec in this case) and transformed them both to RA/DEC coordinates to give two points that can be used to work out what the corresponding rotation in RA/DEC space is. As such 1 arcsec is an arbitrary choice; small enough that it's guaranteed to be within the MRS field of view for which the distortion transforms are defined, and large enough that numerical precision on the coordinate transforms doesn't dominate the measured rotation angle.
So in that diagram, the only issue is that alpha=0 isn't the edge of the field, it's the middle.
As to why I chose to use beta=-1 for the secondary point, instead of +1, this is because the alpha/beta frame handedness is flipped relative to RA/DEC, so the rotation is the angle of the -beta axis measured E from N.
Comment by Jane Morrison: [~dlaw] I just pushed over some new code. The align along alpha beta is not working correctly. Below is an image of three cubes: left: ra-dec, middle: de-rotated align alpha-beta, right alpha-beta cube. I used only 1 file to make the cubes so I was thinking the de-rotated and alpha-beta cubes would look similar. But they do not. Something is wrong about determining the rotation angle - not sure what it is though. !Screen Shot 2020-05-22 at 2.00.19 PM.png|thumbnail!
Comment by David Law: Hm, I'll see if I can take a look early next week. If you're using test data that I messed with the WCS headers to mock up a non-zero roll it's possible that the problem is in how that was done rather than how cube build is working.
Comment by Jane Morrison: [~nluetzgendorf] [~muzerol][~sargent] I am looking into how to make de-rotated cubes (cubes aligned with the slicer plane) for NIRSPEC. The first thing I wanted to do was to see if I could an ifu cube from a single file aligned with the slicer plane using a similar method that MIRI does (coord_system = alpha-beta). Instead of going to ra-dec the cubes are created in the slicer plane. Nadia added the transform to stop at the NIRSPEC slicer plane a while ago. I want to test that - do you have a good example of data - maybe CV3 data - that I could use to these this for NIRSPEC. A source (point source like) might be the best data.
Comment by David Law: [~morrison] I took a look at the derotated_cubes branch and I think I've run down the problem; around line 1108 of ifu_cube.py the declination term isn't being converted from degrees to radians before applying cos(dec) to the delta-ra computation.
The xi=-xi flip in coord.py also needs to happen before the rotation matrix is applied.
Finally, it looks like the rotation information isn't being recorded into the output PC matrices of the cube yet, so I added
if (self.rot_angle is None): self.rot_angle = 0. ifucube_model.meta.wcsinfo.pc1_1 = -np.cos(self.rot_angle np.pi / 180.) ifucube_model.meta.wcsinfo.pc1_2 = np.sin(self.rot_angle np.pi / 180.) ifucube_model.meta.wcsinfo.pc1_3 = 0
ifucube_model.meta.wcsinfo.pc2_1 = np.sin(self.rot_angle np.pi / 180.) ifucube_model.meta.wcsinfo.pc2_2 = np.cos(self.rot_angle np.pi / 180.) ifucube_model.meta.wcsinfo.pc2_3 = 0
to the end of ifu_cube.py
If I make these three changes then I get derotated cubes that look about right to a quick inspection.
Comment by James Muzerolle: [~morrison] Yes, we do have an IFU exposure taken with an external point-like source from CV3 (PRISM only). I'll try to dig it up and send it to you in pipeline-ready format later today.
Comment by Jane Morrison: [~dlaw] I made those changes and still it is not correct. I will continue to look for a mistake. I have pushed the code over. Have you made IFU cubes aligned with the local MRS plane using this method ? Below is the rotated cube (left) on the right alpha-beta cube.
Comment by David Law: Hm, looks like the updated code is working for me. I just pulled over your new update, and below are the cubes that it's creating for a single exposure from that set. Left panel is with 'world' frame, right panel with 'align_slicer'. Which exact file are you using?
!cubes.png!
Comment by Jane Morrison: I am using: det_image_seq1_MIRIFUSHORT_12LONGexp1_cal.fits I believe it came from /ifs/jwst/wit/miri/cubetesting/MIRISim/TEST-SIM-10/
Comment by David Law: I've been using the 1A example, will take a look at 1C
Comment by James Muzerolle: [~morrison] you can find level 2a files here:
/grp/jwst/wit4/nirspec/Muzerolle/for_jane. If for some reason you'd rather have the level 1b files, let me know. Since this is a PRISM exposure, the NRS2 file contains no useful data, but it's there for completeness.
Comment by David Law: I'm getting good results with det_image_seq1_MIRIFUSHORT_12LONGexp1_cal.fits as well
Comment by David Law: Cube building results for det_image_seq1_MIRIFUSHORT_12LONGexp1_cal.fits:
!cubes2.png!
Comment by Jane Morrison: ok strange. I don't know why when I run it does not align with slicer. Did you grab my branch ? Your align slicer values do seem about 180 from alpha-beta values - do you understand that ? I think to make this useful the align-slicer cube needs to match the alpha-beta cubes.
Comment by David Law: I deleted all of my local modifications and pulled your updated branch so it should be identical to your code now. I believe there are two reasons at play for the different orientation of the alpha-beta cubes: First, we're aligning with -beta instead of beta, since at roll=0, beta lies nearly along the North direction (and +V3). Second is the convention that East (and V2) increases to the left in our data cubes by construction, whereas alpha (+alpha is along +V2) increases to the right in the alpha-beta cubes.
For commissioning purposes any flips like this don't matter; the primary purpose is to align the axes so that the along-slice and across-slice behaviour can be characterized without needing to resample/rotate the cube.
I'm inclined to keep the align_slicer cubes as similar to WORLD cubes as possible, since for the majority of people the distinction between +alpha/-alpha and +beta/-beta is unimportant (and those for whom it matters will generally be more on top of the coordinate frames), while introducing flips in the handedness of the coordinates might be more likely to cause issues for the general user. Thoughts?
Hum - I ran it again it works. Maybe I looked at an old result.
I would like to confirm with the MRS WG that it is ok with them that these cube are 180 off the alpha-beta cubes. For ground testing the alpha-beta cubes were sent to a centroider and the center of the point sources in the cube were used for refining distortions and checking alignment. I have not kept up with what these types of cubes will be used for Commissioning - but if the MRS team is going to use them I want to avoid them having to do extra flips later on. I think as long as we are very clear it is fine they there is a 180 - but it would be worth double checking.
On a slightly different note I am now trying to make alpha-beta type cubes for NIRSPEC. I am going to have cube_build be able to make alpha-beta type cubes from single exposures and align_slicer cubes too. If for no other reason just as a double check. But I think NIRSPEC really wants to have true alpha-beta type cubes. I need to change the coord_system from alpha-beta to something more generic. What about 'arrange_slice' ? The slices are stacked together - does that make sense ? For MIRI these cube will always have dimension 2 = number of slices and for NIRSPEC dimension 1 = 30. Make sense ?
Comment by Jane Morrison: [~dencheva][~muzerol][~dlaw] [~nluetzgendorf] I am trying to create IFU cubes in the NIRSpec slicer plane. Nadia has created the transform I need to convert x,y detector values to slicer plane values. It seems to work fine. But I am confused on the units. The picture below is for Prism data (3 X 3 arc seconds - right). I have taken 3 slices on the detector and transformed them to the slicer plane. The along slice direction covers ~ range of 0.012 what are the units ? Do these units make sense to you ? !Screen Shot 2020-05-27 at 2.45.21 PM.png|thumbnail!
Comment by James Muzerolle: [~morrison] That looks as expected. The units are in meters, relative to the center of the IFU aperture.
Comment by Jane Morrison: meters - ok that surprised me. When creating an IFU cube in the slicer plane we still need to define a coordinate system that will work say with ds9.. How do you want this slicer plane coordinate system to be defined. I would say it is usually 0,0 in the center of the IFU ? I was thinking something like - along slice, across slice dimensions. We still need a cdelt1, cdelt2 defined. We need a pseudo wcs like system -
Comment by Jane Morrison: ok I added more slices to fill out the entire IFU region better. Center at 0,0 !Screen Shot 2020-05-27 at 3.45.36 PM.png|thumbnail!
Comment by James Muzerolle: I think the numbering is off in the new plots - even-numbered slices should be at positive values along the x-axis, and the odd-numbered slices should be at negative values. Also, I'm surprised that the range of values in the x-direction for a particular slice is so narrow. Are you plotting the positions for every pixel in each slice? There shouldn't be any gaps between adjacent slices. (see plot below)
!Screen Shot 2020-05-27 at 8.12.39 PM.png!
I don't know what the proper sampling should be for the output. We may have to iterate to find something that doesn't leave holes but also doesn't lose information?
Comment by Jane Morrison: Ok this looks better in terms of ordering of the slices: even slices are in reds/oranges and odd slices are in blue/green
But there is a problem with the mapped x,y values to the slicer plane. I am only getting a single line of values even though I am mapping all the x,y detector values in the slice. When I print out the min and max mapped slicer plane x values it is the same value. I will look more into if I am making a mistake or the transform is not working as expected. !Screen Shot 2020-05-27 at 6.16.52 PM.png|thumbnail!
Comment by James Muzerolle: Ignore what I said about the spread of values in the x-direction... apparently I was up too late to think clearly! What you're seeing is correct - for dispersed data, the model assumes that a source is centered in a slice in the dispersion direction for the purposes of the wavelength calculation, so there should be a single value for the entire length of a slice.
The slice ordering in the new plot looks right.
Comment by Jane Morrison: That makes sense (after I thought about it for a while). For IFU cubes made in the slicer plane I force a 1 to 1 mapping between the slice # and the x coordinate in the slicer plane. So that means that every NIRSPEC IFU cube made in the slicer plane will have 30 bins (for the across slice dimension) or the x dimension. So I was thinking for the example I gave the across slice direction covers ~ -0.0116 to 0.0116 (meters). So the cdelt1 could be (0.0116*2)/30 = 0.000774 meters/cube pixel. We could start of setting the cdelt2 = cdelt1 and see what the ifu cube looks like. Strange units - but I am guessing you want to keep it in units of meters when making IFU cubes in the slicer plane ?
Comment by David Law: [~morrison] After sleeping on it I realize that we don't actually need to worry about handedness flip confusion for end-users; putting +beta upwards flips one direction, but putting +alpha to the right while +RA is left introduces another flip that evens out the handedness issue. Plus, while simulated data is all roll=0, real observations will have a semi-random distribution of roll angles. I've discussed with Alvaro and Javier, and we're now all in agreement. If you just change -2 to +2 on line 1117 of ifu_cube.py everything should come out as expected.
Comment by David Law: Regarding the NIRSpec cubes; is this for the align_slicer cube building option or for alpha-beta? It seems like it would be incredibly confusing to the end user to have align_slicer produce a fully-valid world coordinate WCS (just with a different rotation zeropoint) for MRS, but to produce something in units of meters for NIRSpec. Linear units for 'alpha-beta' cubes from single exposures is still a bit unusual, but this would be a generally not-recommended method of building cubes anyway.
Comment by Jane Morrison: This is for the alph-beta type cubes. Which I need to change the name of since alpha-beta corresponds to MIRI not NIRSPEC. I am going to change the name of coordinate system from 'alpha-beta' to 'align-slices' since that is what we are doing. To not be confused with 'align-slicer' which is in a valid WCS I need to change the current 'align-slicer' which was just temporary until I could think of something better to 'rotate-slices" - what do you think [~dlaw] ? The 'align-slices' (formerly alpha-beta) type cubes are limited to single exposures (in MIRI single channel) are will likely only be used during commissioning. I am thinking they might help de-bug some of the problems we are having with the NIRSPEC IFUs cubes. Once I get these 3 type of cubes I can not imagine we will need any more.
Comment by David Law: Hm, naming, always tricky. Perhaps then we would have these options for coord_system:
'SKYALIGN': This is what's currently called 'WORLD', in which North is up and East left. Works with dithered and single exposures.
'IFUALIGN': This is what's currently called 'align_slicer' in which there is a fully valid RA/DEC world-coordinate WCS, but the zeropoint of the rotation is set such that the cube x/y axes match the slicer direction. Works with dithered and single exposures.
'DEBUG_LOCAL': This is what's currently called 'alpha-beta' in which it uses the local internal coordinate system. For MRS this is the true alpha-beta cubes, for NIRSpec it's whatever their slicer plane coordinate system is called. This only works on individual exposures, and does not have WCS that maps to RA/DEC. These are intended only for debugging during commissioning. We could also just call this 'LOCAL', but 'DEBUG_LOCAL' makes very clear that this isn't a mode intended for general use.
Might be worth a separate conversation about whether SKYALIGN or IFUALIGN is the pipeline default.
Thoughts?
Comment by Marshall Perrin: Speaking as an end user, I concur with David about the confusing-ness of having any WCS in meters. And speaking as an optical scientist, I'll point out we can use the plate scale at the slicer plane to convert from physical units at the focal plane to arcseconds on the sky. The necessary coordinate conversions are in the SIAF, for instance. It certainly seems preferable to me to have consistent units for the WCS between any of the SKYALIGN, IFUALIGN, or DEBUG_LOCAL options. At the very least, consistent between the first two of those.
Comment by James Muzerolle: I think we're talking about two different products that have two different use cases. The NIRSpec version of an "alpha-beta" cube, which corresponds to our slicer optical plane, is needed primarily for calibration, so that we can create cubes and extract spectra for our internal lamps. Most users probably will not need to make use of that, as opposed to the second version, the "align-slicer" cube, which is just sky coordinates but with the rotation removed so that it is aligned with the internal slicer plane orientation. I think this is what is needed for cases such as PSF subtraction?
Since the slicer is an image plane within the instrument, the model transforms are set up to produce positions in physical size units; the IDT likes SI, so in this case it's meters. We could apply a plate scale to convert to angular units, though that's not really necessary for internal calibration data. The question is whether this would be something of interest to users looking at on-sky data?
Comment by James Muzerolle: I like David's suggested names, except for 'DEBUG_LOCAL' since that is not optional for NIRSpec. How about something like 'INTERNAL_CAL'?
Comment by David Law: INTERNAL_CAL works for me
Comment by Jane Morrison: Thanks it sound like we have a consensus - SKYALIGN, IFUALIGN and INTERNAL_CAL
James just double checking - for NIRSPEC - INTERNAL_CAL type IFU cubes. Will these cube be constructed from a single exposure ?
Comment by James Muzerolle: [~morrison] yes, INTERNAL_CAL cubes will only be constructed from individual exposures. We will not need to combine multiple exposures for any internal lamp calibration data.
Comment by Jane Morrison: [~dencheva][~muzerol] There might be an error in the NIRSPEC transform 'detector' to 'slicer' Here are the steps I do slice_wcs = nirspec.nrs_wcs_set_input(input, i) # i = slice number x, y = wcstools.grid_from_bounding_box(slice_wcs.bounding_box, step=(1, 1), center=True) detector2slicer = slice_wcs.get_transform('detector','slicer') coord1,coord2,lam = detector2slicer(x,y)
The wavelengths returned (along the slice) are all close to zero for each slice. Now I have gotten around this problem by also mapping the x,y values to the sky using the detector to world transform: c1, c2, lam = slice_wcs(x, y) I use those wavelength values which are fine. I just wanted to mention this incase this is not how the detector to slicer transform is supposed to work
Issue JP-1013 was created by David Law:
In order to support commissioning activities to determine the MRS PSF (known to be different in the alpha-beta directions) it will be necessary to construct alpha-beta cubes for dithered observations. These are cubes in which the along-slice and across-slice directions are aligned with the X/Y spaxel axes of the cube.
This has been on the long-term agenda of the MIRI team for a while, but was pending an easy solution of how it might be implemented that is now resolved.
Essentially, what is necessary is a data cube with astronomical RA/DEC coordinates in which the rotation of the cube is set such that the along-slice (alpha) direction is along the X-axis of the cube. This can be achieved simply by applying a rotation angle Theta when computing the Xi-Eta pixel coordinates from the input RA/DEC coordinatates, where Theta is defined as the position angle of the along-slice direction in the first exposure (computed using the embedded wcs). Specification of a keyword when running cube_build would thus determine whether the position angle of the cube is 0 (which it always is at the moment) or allow it to be aligned with the IFU hardware. Both kinds of cubes would be equally astronomically valid.
Technical note: alpha may not increase to the right, and beta may not increase up in these cubes. The alpha/beta coordinate system contains some flips relative to V2/V3 (and by extension RA/DEC) that should not be reproduced so that these cubes are simply rotated with respect to the standard PA=0 cubes. This may mean, for instance, that beta increases downwards.