LSSTDESC / descwl-shear-sims

simple simulations for testing weak lensing shear measurement
BSD 3-Clause "New" or "Revised" License
10 stars 9 forks source link

WIP: implement a simple z-dependent shear #128

Open NiallMac opened 3 years ago

NiallMac commented 3 years ago

Allow for a shear which takes a different value within a given redshift interval. Galaxy catalog needs to have a _get_redshift function.

No real testing implemented as of yet. I think there are two ways we'd want to test this: i) objects are getting the correct (z-dependent) shear at the truth catalog level (we need to implement truth catalogs for this) ii) we measure objects (in an idealized sim setup) to have the correct (z-dependent) shear - I need to understand how the measurement steps work in order to do this.

But suggestions for simpler unit tests welcome ofc.

esheldon commented 3 years ago

Thanks Niall.

This looks reasonable at first glance.

I won't get a chance to review in detail for a bit due to the desc meeting.

I do recommend getting it to pass flake8 so it will then pass continuous integration. Also we need some unit tests.

NiallMac commented 3 years ago

I fixed the flake8 stuff and added a unit test. The unit test is very simple (in test_zslice_shear.py), and just tests the ZSliceShear class returns shear values correctly, rather than using it in a simulation. I can try and add something that does the latter also. To test that properly, I think we'd actually want to simulate from a catalog with a range of redshifts, and do some shape measurement to ensure objects are getting the correct shear for their redshift. Are there unit tests already that test for shear recovery that I can work from?

esheldon commented 3 years ago

Thanks Niall. I see the struggles with flake8; I usually find it easier to run flake8 myself from the root directory to fix this stuff (or do it from your editor). It can be a frustrating thing waiting for the CI to do it.

Yes, a simple test would be a simple low noise, low number of objects grid test, with a catalog that just has one redshift. Then we could do a grid with two redshifts; since you feed the catalog you should be able to find the objects with each redshift type and check the result

We also will need a full integration test, which will take more thought.

NiallMac commented 3 years ago

Lol, yeh sorry about the flake8 commit traffic - I hadn't used it before but after several attempts I thought to myself there must be a better way to do this, and realised you could run it locally 🤦 .

On the test...it's not clear to me in this context what it means for an object to have a redshift. Would it make sense to define a new RedshiftGalaxyCatalog class where objects have a redshift also? In the PR, I added a def _get_redshift(self, i) function to WLDeblendGalaxyCatalog - should the presence/absence of this function indicate whether a catalog has redshift information? We could also think about whether we want the redshift to be an attribute of our objects - that would require extending the currently used GSObject a bit I think, to allow for extra attributes like redshift. There may be some overlap here with how we want to keep track of truth information e.g. the index of a given object in the input catalog.

When you say "find the objects with each redshift type and check the result" I think you mean check that they have the correct shear for their redshift? This is what I was suggesting above, and I was asking about whether there is shape measurement already implemented in some of the other tests that I can use as a guide.

beckermr commented 3 years ago

We can't test shape measurement directly in the unit tests. It takes too long. I think the most basic test would be to put objects on a grid with half sheared and half not sheared, run mdet, match back to the truth catalog, and then test that the sheared ones have m of order few times 10^-4. My student is working on this test exactly in the old mdet sims code base now.

esheldon commented 3 years ago

We can test the shapes for low noise; basically we can use zero noise, generate the image and measure the shapes and make sure we get out what we expect. "What we expect" can be just what we get when we first run the test :) Then it is a regression test, but it is only OK if we first show that everything is fine with an integration test.

esheldon commented 3 years ago

Really the full shear recovery test is combined integration and validation test

NiallMac commented 3 years ago

@esheldon agreed - so back to my question :) - is there an example of shape measurement in any of the existing tests that I can work from?

If not, I could just run galsim's hsm? Is there a way to access the positions of the simulated objects to pass to the shape measurement? Or would I need to calculate these based on the grid geometry?

NiallMac commented 3 years ago

@esheldon or @beckermr I could use more feedback here on the questions I've asked. As in my original comment it's not what we want to test here that I'm asking about really, but how. I'm not familiar with this codebase and don't want to waste everyone's time by attempting to implement something in a way that does not fit with your vision.

In order to test this code and move forward we need i) truth information associated with a simulation i.e. a catalog from which the user can infer what was simulated where. ii) we need to feed the simulation into some sort of shape measurement to test if objects received the correct shear, which also means knowing the coordinates and assigned shear of the simulated galaxies, and so relates to i).

So, do you have thoughts/preferences/plans/advice on the implementation of (i), and do you have any pointers or examples of running shape measurement on these sims that can help me with (ii)?

esheldon commented 3 years ago

We don't have any tests like that in this package yet. We usually deferred things like that to packages down stream. We could put the test in a downstream package or add it here, either way is fine. Let's discuss options

We would need a way to get "truth" info out for tests not on a grid, which is a separate issue.

I propose a simple test using a grid. Make a catalog class that generates round objects on a grid, and gives objects on one side of the grid a certain redshift and no shear and on the other side of the grid a different redshift and a shear. Then run sep to find objects, and check that the average shape for the two halves looks as expected.

This adds a dependency on sep, but this can be checked for in pytest and skipped if it isn't there.

mr-superonion commented 1 year ago

Hello, I think this implementation is very important!! I really wish to use this z-dependent shear functionality in the code!

Is there anything I can do to help on the pull request?

beckermr commented 1 year ago

Feel free to take it over and finish it.

mr-superonion commented 1 year ago

Sorry for being silent for a while, I am starting to make a shear object and then we can put shear distortion from different cases to the shear object. Please see the class for z-dependent shear: https://github.com/mr-superonion/descwl-shear-sims/blob/16a2fe512569baf3803fb5c447013e852854a1a7/descwl_shear_sims/shear.py#L99

I am planning to use a trinary number to denote the state of each plane (whether it is positive, negative or not) distorted. In addition, I record the redshfit information from WLDeblendCatalog (see here).

Those updates happen in https://github.com/LSSTDESC/descwl-shear-sims/pull/198.