Allow independent handling of noshear in metacal

[sidneymau]

Proposed solution for #250. Rather than forcing the processing of 1p if noshear is specified, this updates the code to check

1. if only noshear is requested, then process that independently
2. if noshear and other types are requested, if noshear has 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)

[sidneymau]

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)

[sidneymau]

The code here reproduces some of the internals of get_obs_galshear but extracting the noshear branch. Maybe the function can be broken up so we things stay consistent with any future changes?

[sidneymau]

My bad, didn't catch the existing test. Updated accordingly

[esheldon]

Hi Sid. I appreciate the PR. However, this is a breaking change.

What's your motivation for this change?

[sidneymau]

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 noshear branch, 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 1p were requested alongside noshear, it would be more efficient to process noshear alongside that other shear rather than also adding 1p (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

[sidneymau]

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

[esheldon]

You can call get_obs_galshear directly.

[esheldon]

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.

[sidneymau]

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)

[sidneymau]

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)

[beckermr]

It is breaking. I'd be ok supporting a "noshear-only" version that really is only "noshear".