Allow independent handling of noshear in metacal
Proposed solution for #250. Rather than forcing the processing of 1p if noshear is specified, this updates the code to check
- if only
noshearis requested, then process that independently - if
noshearand other types are requested, ifnoshearhas not yet been run, process that alongside another type
The code produces the desired behavior but there are no tests yet. I'm happy to implement one (probably something that quickly runs metacal with a few different types requests and checks that the output dict has the same types as requested)
There may need to be some additional handling in case the requested types include noshear and psf types but not normal shear types (in which case len(types) > 1 would be True, but there would never be an opportunity to process noshear)
My bad, didn't catch the existing test. Updated accordingly
Hi Sid. I appreciate the PR. However, this is a breaking change.
What's your motivation for this change?
There are a few motivations:
- It would be convenient for testing the detection & fitting pipeline from metadetect in a simplified case. For example, I want to compare the detections and fit fluxes to a truth catalog for some simulated image. This can be accomplished with just the
noshearbranch, and running other shears adds substantial compute time (the limiting step is the metacal process, so it takes longer for each shear type processed) - It seemed weird to have different shear types processed than were requested, and
- If shear other than
1pwere requested alongsidenoshear, it would be more efficient to processnoshearalongside that other shear rather than also adding1p(e.g., if the user wanted to process[noshear, 2p, 2m]then it would be inefficient to process 4 types instead of 3 types)
So the motivations are mostly efficiency with some aesthetic
edit: I also first thought I had a bug in my code because I was getting a different set of shear types in my output than I requested in my config, so I had to spend some time debugging before I realized that this was default (but undocumented, as far as I can tell) behavior
The alternative I was coming up with was to reconstruct the MEDSifier to get the detection stage run on the mbobs but then I may be missing some of the PSF transformations and running the fitters seemed more complicated
You can call get_obs_galshear directly.
I guess if you are using get_all_metacal it needs get_all() method to change
The reason this exists is that noshear/1p can share some calculations, and this improves the speed when you are planning to get more than just noshear.
If anyone relied on that it will break.
In this case I am running code from metadetect (i.e., metadetect.do_metadetect and/or using the Metadetect class) -- that code runs get_all_metacal which runs get_all, which is how I arrived here.
Part of my thinking was that noshear can share those calculations with any other shear type, so you can have the calculations be shared between noshear and the first other shear type requested (not just 1p)
I do appreciate that it is bad to break standing behavior -- but if the decision is to keep this as-is I do think it would be good to at least document it throughout (none of the docstrings in any of the functions mention this behavior but they will all fall back to it)
It is breaking. I'd be ok supporting a "noshear-only" version that really is only "noshear".