kammerje / spaceKLIP

Pipeline for reducing JWST high-contrast imaging data. Published in Kammerer et al. 2022 and Carter et al. 2022.
https://ui.adsabs.harvard.edu/abs/2022SPIE12180E..3NK/abstract
MIT License
16 stars 9 forks source link

Problems highpass filtering data with calibrated contrast curves #148

Open maxwellmb opened 6 months ago

maxwellmb commented 6 months ago

If a user were to pass in the highpass keyword to pyklip, they might also want this to propagate to the calibrated contrast curve inject, but at the moment, on this line in Analysistools, highpass is hardcoded to false. Is there a reason for this or can we let it be the same is what was done in the PSF subtraction call to pyklip?

maxwellmb commented 6 months ago

Relatedly, it looks like the kwargs for pyklip are read in from the database. I believe (but @semaphoreP can correct me) that the pyklip inputs are all saved to the pyklip headers, and so I wonder if it might be safer to read them all from there when things are loaded in again e.g. in line 529.

kammerje commented 6 months ago

Hi @maxwellmb. Thanks a lot for bringing up this issue! Please note that high-pass filtering is currently not supported (with the develop branch). We are working on a fix for this (see dev/jk branch). However, this will also require updates in the pyKLIP code, see comment here. We are planning to merge this fix as soon as the updates on the documentation have been completed.

semaphoreP commented 6 months ago

Max just made this PR to pyklip (https://bitbucket.org/pyKLIP/pyklip/commits/2138fc075f9fe201f4a8efbb3ac0a892514d9cea), @kammerje should check if this is the same behavior he is aiming for or different. (Basically, whether to filter the master library on construction, or at the klip_dataset level [which Max is proposing])

kammerje commented 6 months ago

@semaphoreP Ok, I will check! Does this also apply the high-pass filtering to the KLIP FM dataset?

semaphoreP commented 6 months ago

Not currently. we would need to do the same thing here. I think we should just be consistent between the two of you, and not have 2 different sets of behaviors for how to high pass filter the reference library

kammerje commented 6 months ago

For now, I always applied the high-pass filter when generating the RDI library with pyKLIP. This ensures that PSF subtraction and FM behave consistently. Everything else is then spaceKLIP business, right?

semaphoreP commented 6 months ago

I think that makes sense, but is not the behavior @maxwellmb proposed in his pyklip PR, so we should see if he's ok with that

maxwellmb commented 6 months ago

Alright, I've implemented a version like Jens' suggestion, see the commit here. I tested it with a hacked up version of spaceklip of mine, not with dev/jk (since that requires a bunch of updating things for me), but it produces the desired behavior, including throwing an error if klip_dataset is called with a different highpass value than what was used when reading in the rdi dataset. @kammerje, does this meet your needs?

kammerje commented 6 months ago

Yes, this is exactly what I had done too!

semaphoreP commented 6 months ago

Great! @kammerje should I accept Max's pyklip PR, or do you want to merge any additional changes you made into it?

kammerje commented 6 months ago

@semaphoreP Yes, please do so, this is exactly what I need for my spaceKLIP branch to work!

semaphoreP commented 6 months ago

Ok merged those changes in pyklip!

AarynnCarter commented 3 weeks ago

@maxwellmb is this resolved now?