Implementing Calibrated Contrast Measurements #91

AarynnCarter · 2023-08-11T17:19:56Z

Putting together all the code necessary to perform calibrated contrast measurements.

Implement masking for MIRI FQPM's so that raw contrasts can be computed
Restructure pyklippipeline.py so that pyKLIP datasets can be created more easily
Implement companion injection into pyKLIP datasets using existing pyKLIP functions
Implement calibrated contrast calculation

Resolves #79, Resolves #85

Started by implementing the MIRI FQPM masking, this is improved from original spaceKLIP and uses rectangles instead of pizza slices.

AarynnCarter · 2023-08-11T21:12:10Z

The SpaceTelescope(Data) class that was in pyklippipeline.py has now been removed. Instead, I have ported this over to pyKLIP itself under a new jwst_new branch: https://bitbucket.org/pyKLIP/pyklip/src/jwst_new/pyklip/instruments/JWST.py.

Important: We shouldn't merge this spaceKLIP branch until we are happy with the pyKLIP file / have merged things into pyKLIP's jwst branch.

EDIT: Don't worry about failing checks at the moment, they are related to the code wanting to use the older version of pyKLIP.

AarynnCarter · 2023-08-17T22:43:42Z

Companion injection and recovery is now coded up, based on fitting 2D Gaussians to estimate the peak flux of injected sources after KLIP subtraction. For MIRI this is probably okay, but perhaps we should do something smarter for NIRCam in the future.

AarynnCarter · 2023-08-18T18:24:13Z

Okay calibrated contrast curves can now be calculated. I won't merge in just yet, as I think further thought needs to go into retrieved throughputs >1. Plus, it would be great if a few people could test this first. @mperrin, @kammerje, I know you are on vacation right now, but perhaps when you get back you could review what I've done.

AarynnCarter · 2023-08-25T20:35:14Z

Added some minor bug fixes.

Resolves #88, Resolves #92

mperrin · 2023-08-28T18:15:52Z

@AarynnCarter FYI I'm back from vacation, still playing catchup on many things. I see & acknowledge your request for me to take a look at this, and will try to do so within the next ~2 weeks (i.e. before the conference in a couple weeks here).

Is this "ready to go" from your perspective? I see it's still a Draft PR. And can you remind me if there's a plan yet for timing re the related pyKLIP branch merges etc?

AarynnCarter · 2023-08-28T18:53:24Z

@mperrin This should be good for review, yes. I might add some minor bug fixes in if needed. This is a somewhat complicated PR as we'll need to make sure pyKLIP is updated first, so once you think it's good to go I will coordinate with Jason so that everything gets updated together.

AarynnCarter · 2023-09-20T21:37:59Z

@AarynnCarter Add logging for calibrated contrast steps

mperrin

Apologies for the delay in this -- I started this review many weeks ago but got interrupted and didn't get back to it until now.

The comments are mostly suggestions for clarification, possible improvement in parts. I don't see any show stoppers to merging this, but I think a round of revisions would be beneficial, so this review is for now just a "comment", neither an approve nor a formal request for changes.

mperrin · 2023-09-20T20:37:31Z

requirements.txt

@@ -4,5 +4,6 @@ jdaviz
 jwst
 matplotlib
 numpy
+pysynphot


I would urge you to reconsider this dependency, since pysynphot is deprecated and no longer actively developed. Instead synphot should be preferred as the current tool, with similar but slightly different API after a refactoring. synphot is actively developed and supported.

And yes, this is an annoyance from upstream, but wanted to make sure you were aware.

Looks like this is tied to the psf.py file, which is built upon WebbPSF_ext. I 100% agree we should change, but don't want to bloat this PR further / I think this will need a careful hand to make sure we don't break anything. Instead, I've made a new issue so that we don't lose track.

mperrin · 2023-09-20T20:40:16Z

spaceKLIP/analysistools.py

@@ -121,6 +124,16 @@ def raw_contrast(self,
        output_dir = os.path.join(self.database.output_dir, subdir)
        if not os.path.exists(output_dir):
            os.makedirs(output_dir)
+
+        # Copy the starfile that will be used into this directory
+        starfile_type = starfile.split('.')[-1]


minor: use os.path.splitext for this.

mperrin · 2023-09-20T20:44:05Z

spaceKLIP/analysistools.py

+        new_starfile_path = output_dir+'/user_starfile.'+starfile_type
+        header = '#'+starfile.split('/')[-1] + ' /// {}'.format(spectral_type)+'\n'
+        with open(new_starfile_path, 'w') as new_starfile:
+            with open(starfile, 'r') as orig_starfile:
+                new_starfile.write(header)
+                for line in orig_starfile.readlines():
+                    new_starfile.write(line)


Minor: I see what you're doing here, I guess, but this seems kinda a low-level kludge, to copy a data file line by line just to append a comment text at the start. This is a minor point and I'm OK if you want to leave as-is, it just strikes me as an unnecessary complication, and from a code style perspective is an interruption that breaks up the conceptual flow, and breaks layers of abstraction. Consider encapsulating this sort of thing into utility functions.

Moved function to utils.py, streamlined it a bit, and added an error flag if the file can't be found.

mperrin · 2023-09-20T20:49:27Z

spaceKLIP/analysistools.py

+                    nanmask[nanmask<0.5] = 1
+
+                    # Downsample, remove padding, and mask data
+                    nanmask = nanmask[::samp,::samp]


I don't follow the logic here about oversampling. You make the mask oversampled by 15x, but here you downsample to every 15th sample, without any averaging at all. So what is the point of the upsampling? The vast majority of the upsampled pixels (1 - (1/15)**2, so 99.1%!) are discarded without ever being used.

The sampling is for the nanmask, so there isn't really any averaging to do. A pixel should either be == 1 (not masked) or == np.nan (masked). I found that by upsampling the array initially, the rectangles that I apply can be better centered in the image, perhaps I could adjust how the rectangles are being made to remove the need to upsample but it seems to work right now and it's relatively quick.

However, by looking at this I spotted that some pixels, despite being surrounded by NaNs, weren't being masked. I've added a new utility function to catch these and fix them.

mperrin · 2023-09-20T20:57:14Z

spaceKLIP/analysistools.py

@@ -269,6 +339,7 @@ def raw_contrast(self,
                        ax.plot(seps[k], cons[k], color=colors[k % mod], alpha=0.3)
                        ax.plot(seps[k], cons_mask[k], color=colors[k % mod], label=klmodes[k] + ' KL')
                ax.set_yscale('log')
+                ax.set_ylim([None,1])


A general comment on the raw_contrast plots (not this particular line of code): When running in a notebook, it would be nice if the PDF plots are displayed in the notebook, as well as saved to disk. At minimum, the text in the function that writes out like Contrast results saved to [filename etc] should also state that PDF plots were saved into that directory.

The point here is to help make it more discoverable and obvious to users that these files now exist, and where to find them...

I'm not familiar enough with notebooks to understand why things aren't being plotted there, maybe that's best left for a future improvement? I did add a quick adjustment to the print statement that mentions plots have been saved.

mperrin · 2023-12-07T04:39:01Z

spaceKLIP/analysistools.py

+                plt.savefig(save_string + '_calcon.pdf', 
+                            bbox_inches='tight', dpi=300)
+
+        pass


pass is unnecessary here

mperrin · 2023-12-07T04:39:52Z

spaceKLIP/analysistools.py

+    raw_dataset : pyKLIP dataset
+        A pyKLIP dataset which companions will be injected into and KLIP
+        will be performed on. 
+    true_companions :


doc string missing info for true_companions

mperrin · 2023-12-07T04:40:03Z

spaceKLIP/analysistools.py

+
+    Returns
+    -------
+    TBD


mperrin · 2023-12-07T04:44:18Z

spaceKLIP/imagetools.py

-                head_pri['XOFFSET'] = xoffset
-                head_pri['YOFFSET'] = yoffset
+                head_pri['XOFFSET'] = xoffset / 1e3 #Should be saved in arcseconds for FITS file
+                head_pri['YOFFSET'] = yoffset / 1e3 #Should be saved in arcseconds for FITS file


Ideally this ought to include a FITS header comment giving the units!

head_pri['XOFFSET'] = (xoffset, '[arcsec] X offset in arcseconds') head_pri['YOFFSET'] = (yoffset, '[arcsec] Y offset in arcseconds')

(See FITS standard, section 4.3.2)

/ 1e3 was conflicted with the develop branch and has been removed (hence the reason I was confused why it was there). Totally agree on FITS header comments, but this is an issue that extends beyond the XOFFSET and YOFFSET values - we haven't done the best job at including these. I'll make a separate issue to address this code wide.

mperrin · 2023-12-07T04:46:05Z

spaceKLIP/imagetools.py

@@ -1988,9 +1991,10 @@ def align_frames(self,
            f = plt.figure(figsize=(6.4, 4.8))
            ax = plt.gca()
            for index, j in enumerate(ww_sci):
-                ax.scatter(shifts_all[index][:, 0] * self.database.obs[key]['PIXSCALE'][j], shifts_all[index][:, 1] * self.database.obs[key]['PIXSCALE'][j], s=5, color=colors[index], marker='o', label='PA = %.0f deg' % self.database.obs[key]['ROLL_REF'][j])
+                ax.scatter(shifts_all[index][:, 0] * self.database.obs[key]['PIXSCALE'][j], shifts_all[index][:, 1] * self.database.obs[key]['PIXSCALE'][j], s=5, color=colors[index%10], marker='o', label='PA = %.0f deg' % self.database.obs[key]['ROLL_REF'][j])


(code style) This line is really dense and hard to read... Consider reformatting over a couple of lines

…alcon

…pdate

AarynnCarter · 2023-12-13T23:49:42Z

Latest commit Resolves #119

… some minor docstring fixes

…ntrast calcs

mperrin

Reviewed, tested extensively, some debugging and additional enhancements to plots added. Discussed in Slack with @AarynnCarter and @kammerje.

Working; looks good to me. 👍

…alcon

AarynnCarter added 3 commits August 11, 2023 10:12

Adding masking of MIRI FQPM edges for contrast curves

a3dc64b

Converting pyfits to fits

a53739a

Removing SpaceTelescope Data class

07f5f4b

AarynnCarter added 2 commits August 15, 2023 16:03

Source injection working for calibrated contrast

abf970b

Adding ability to inject and recover companion fluxes

141e1ab

Adding determination of calibrated contrast curves

a3c6fc9

Minor bug fixes

d4da01a

AarynnCarter marked this pull request as ready for review August 28, 2023 20:08

Small plotting change

92da1df

mperrin mentioned this pull request Aug 29, 2023

Tutorial notebooks and some code improvements for version 2 #84

Merged

3 tasks

AarynnCarter changed the base branch from main to develop August 29, 2023 17:32

mperrin and others added 2 commits August 31, 2023 11:00

Merge branch 'develop' into calcon

8087192

Merge branch 'develop' into calcon

f675740

mperrin reviewed Dec 7, 2023

View reviewed changes

AarynnCarter added 9 commits December 11, 2023 11:55

Merge branch 'calcon' of https://github.com/kammerje/spaceKLIP into c…

d5850be

…alcon

Adjust to use os.path.splitext()

6218033

Adjust to use os.path.splitext()

11fdf6b

Adding function to set pixels surrounded by NaNs to desired value.

2696fb1

Adjusting raw contrast print statement

8365553

converting from hardcoded 6.5 to JWST_CIRCUMSCRIBED_DIAMETER

2379f94

Adding print statement for injection and recovery

6c89911

Minor documentation changes

e754d1b

Reformat ax.scatter() call

8482f7c

AarynnCarter added 2 commits December 11, 2023 14:32

Removing conflicts

1674713

Ensuring everything is in arcseconds

fa44290

mperrin mentioned this pull request Dec 12, 2023

minor fixes & enhancements while testing the tutorial notebook #120

Merged

AarynnCarter added 3 commits December 13, 2023 09:30

Changing to WebbPSF for MIRI filter info

489b2c2

making sure calcon works with latest version of develop + gitignore u…

5a75647

…pdate

Fixing bug when number of different KL modes == 1

a06eb2a

mperrin added 7 commits December 14, 2023 01:14

Merge branch 'develop' into calcon

662113f

Merge branch 'develop' into calcon

5e10701

add more log statements and printed notification of files saved; also…

af0df19

… some minor docstring fixes

debug and enhance plotting code for raw and cal contrast

b5073a0

add plot title annotation of PSF subtraction strategy

f6e903c

trivial bug fix, add missing 'self.'

45b1041

Tutorial updates for calcon branch with working raw and calibrated co…

b525077

…ntrast calcs

mperrin approved these changes Dec 20, 2023

View reviewed changes

AarynnCarter added 2 commits December 21, 2023 08:16

Adjusting install to jwst_new

7b566ac

Merge branch 'calcon' of https://github.com/kammerje/spaceKLIP into c…

3ceda4a

…alcon

AarynnCarter merged commit 2847c9a into develop Dec 21, 2023
2 of 4 checks passed

mperrin deleted the calcon branch February 6, 2024 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing Calibrated Contrast Measurements #91

Implementing Calibrated Contrast Measurements #91

AarynnCarter commented Aug 11, 2023 •

edited

Loading

AarynnCarter commented Aug 11, 2023 •

edited

Loading

AarynnCarter commented Aug 17, 2023

AarynnCarter commented Aug 18, 2023

AarynnCarter commented Aug 25, 2023

mperrin commented Aug 28, 2023

AarynnCarter commented Aug 28, 2023

AarynnCarter commented Sep 20, 2023

mperrin left a comment

mperrin Sep 20, 2023

mperrin Sep 20, 2023

AarynnCarter Dec 11, 2023

mperrin Sep 20, 2023

AarynnCarter Dec 11, 2023

mperrin Sep 20, 2023

AarynnCarter Dec 11, 2023

mperrin Sep 20, 2023

AarynnCarter Dec 11, 2023

mperrin Sep 20, 2023

AarynnCarter Dec 11, 2023

mperrin Dec 7, 2023

AarynnCarter Dec 11, 2023

mperrin Dec 7, 2023

AarynnCarter Dec 11, 2023

mperrin Dec 7, 2023

AarynnCarter Dec 11, 2023

mperrin Dec 7, 2023

AarynnCarter Dec 11, 2023

mperrin Dec 7, 2023

AarynnCarter Dec 11, 2023

AarynnCarter commented Dec 13, 2023

mperrin left a comment

Implementing Calibrated Contrast Measurements #91

Implementing Calibrated Contrast Measurements #91

Conversation

AarynnCarter commented Aug 11, 2023 • edited Loading

AarynnCarter commented Aug 11, 2023 • edited Loading

AarynnCarter commented Aug 17, 2023

AarynnCarter commented Aug 18, 2023

AarynnCarter commented Aug 25, 2023

mperrin commented Aug 28, 2023

AarynnCarter commented Aug 28, 2023

AarynnCarter commented Sep 20, 2023

mperrin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AarynnCarter commented Dec 13, 2023

mperrin left a comment

Choose a reason for hiding this comment

AarynnCarter commented Aug 11, 2023 •

edited

Loading

AarynnCarter commented Aug 11, 2023 •

edited

Loading