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

SMURF responsivity images no longer have FPLANE coordinate frame #40

Closed
aggibb opened this issue Sep 17, 2014 · 7 comments
Closed

SMURF responsivity images no longer have FPLANE coordinate frame #40

aggibb opened this issue Sep 17, 2014 · 7 comments

Comments

@aggibb
Copy link
Member

aggibb commented Sep 17, 2014

/stardev in Hilo is at 3204cbb and produces responsivity files with no FPLANE coordinate frame. Unfortunately this breaks the pipeline (which mosaics them in FPLANE coordinates). Here are the command line arguments used by the pipeline (with suitable dummy names):

% calcflat in=^inlist method=polynomial order=1 resist=^$STARLINK_DIR/share/smurf/resist.cfg \
    respmask=true snrmin=3 resp=resp1 out=flat1

The output responsivity images has only 4 coordinate frames (grid, pixel, axis and fraction) where it should also have BOLO and FPLANE.

The bug was introduced after daf2bb2, as /stardev at the JCMT works fine.

@timj
Copy link
Member

timj commented Sep 17, 2014

Well, if you hadn't told me it worked after this commit I would have put money on the breakage being introduced with 8a7d698 from July.

@dsberry
Copy link
Member

dsberry commented Sep 17, 2014

On 17 September 2014 01:46, Tim Jenness notifications@github.com wrote:

Well, if you hadn't told me it worked after this commit I would have put
money on the breakage being introduced with 8a7d698
8a7d698
from July.

Yes indeed. When I try running calcflat, I find that refdata->hdr->tswcs
is NULL at line 197 of smf_create_bolfile.c. This is because no time-series
WCS is stored in the output smfData created by smf_flat_mergedata. May
be 8a7d698 should be changed to use the old scheme if refdata->hdr->tswcs
is NULL or contains no FPLANE frame ???

David


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

@timj
Copy link
Member

timj commented Sep 17, 2014

I'd prefer it if WCS flowed through the system properly rather than being magically created at the last moment. The problem is that we need to isolate SCUBA-2-specific calls if we hope to use SMURF to reduce data from other instruments. Do the heat frames in smf_flat_mergedata have WCS?

@dsberry
Copy link
Member

dsberry commented Sep 18, 2014

No they don't. Looking into this some more, the smfData returned by smf_flat_fastflat no longer has time as it's third axis, and so its tswcs frameset is left NULL. I think this is where it comes from. Maybe we could change smf_flat_fastflat so that the output smfData has a non-NULL tswcs which retains the spatial axes on the input, and adds a third axis representing heater value. The use of the variable name "tswcs" - which I presume means "time series WCS" - is then a bit wrong, since it's being used to describe something other than a time series.

@timj
Copy link
Member

timj commented Sep 18, 2014

Presumably we could do what we do when reading a 2D image in that we put the 2D frameset in the wcs slot and leave tswcs NULL (also istseries=0). Then we just look for wcs in preference to tswcs. Would that work?

dsberry pushed a commit that referenced this issue Sep 19, 2014
This should fix Starlink issue #40 "SMURF responsivity images
no longer have FPLANE coordinate frame".
@dsberry
Copy link
Member

dsberry commented Sep 19, 2014

Hum - that would then break the expectation that "wcs" is a 2-D FrameSet. In fact, what I've done is to use "tswcs", on the basis that there is a precedent for this in the use of "tswcs" to describe the spectral axis of an FFT. Commit 9750b85 seesm to fix this issue.

@dsberry dsberry closed this as completed Sep 19, 2014
@aggibb
Copy link
Member Author

aggibb commented Sep 19, 2014

Thanks David - it does indeed look like it's fixed as I'm looking at some FPLANE mosaics from the pipeline right now.

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

3 participants