Closed dlanzieri closed 3 years ago
Thanks a lot Denise for opening this draft PR. I had a look at your code, and after a little bit of investigations I think I have found the answers to all the problems. I had to do a little bit of research and tinkered with the code a lot, so I opened a seperate PR with my solution, which reuses some of your code here.
You can have a look there: https://github.com/DifferentiableUniverseInitiative/flowpm/pull/79
You might have one extra test here that I didn't reimplement, the one that just tests the interpolation of the density planes onto sky coordinates, but since I managed to get the convergence maps to agree exactly this is included in the test of the convergence maps.
To summarize.... here are the main issues I found:
This is what I was trying to explain here https://github.com/DifferentiableUniverseInitiative/flowpm/pull/62#issuecomment-810590964 but in your code here: https://github.com/DifferentiableUniverseInitiative/flowpm/blob/5743babce7312615ac5bef95cd10435a155905e4/tests/test_lenstools.py#L131 you were applying a shift which wasn't an offset of half a pixel...
The same issue happens also when we interpolate, because lenstools assumes that the coordinates of the pixels are at the center and not the corner, so again there is a 0.5 pixel offset that needs to be taken into account. I included it in my version here: https://github.com/DifferentiableUniverseInitiative/flowpm/blob/0f029e5e1df91db44ca5dabb9e497a694048491f/flowpm/raytracing.py#L102
When lenstools normalizes the lensplanes, it uses the total number of particles in the snapshot. Whereas we were using the total number of particles in the lensplane. This was leading to a small difference which could translate in several percent changes at the level of the density planes. Here is we were doing this: https://github.com/DifferentiableUniverseInitiative/flowpm/blob/5743babce7312615ac5bef95cd10435a155905e4/tests/test_lenstools.py#L237
Another issue that came up was that at some point, we were recomputing the comoving distance from the valuie of the scale factors, here: https://github.com/DifferentiableUniverseInitiative/flowpm/blob/5743babce7312615ac5bef95cd10435a155905e4/tests/test_lenstools.py#L222 But that was not necessary, as we already had access to these distances that we turn into scale factor here: https://github.com/DifferentiableUniverseInitiative/flowpm/blob/5743babce7312615ac5bef95cd10435a155905e4/tests/test_lenstools.py#L201
So we had a roundtrip in comoving distance, and unfortunately that was really not very accurate, with errors in the 10%. This distance goes into the normalization of the density plane, so a large contribution in the error of the density planes was just due to this.
To make sure that the various normalization factors are working correctly I added a test in my version on the mean of the density planes, which doesnt depend on the painting, and is able to catch any problem with the cosmology or density normalization factors: https://github.com/DifferentiableUniverseInitiative/flowpm/blob/0f029e5e1df91db44ca5dabb9e497a694048491f/tests/test_raytracing.py#L200
Thank you @EiffL for this detailed description. Regarding the first point, I'm afraid in an effort to replace the specific setting of the simulation with the generic variables, I replaced the 0.5 (the offset ) with the field (that was 5.)
positions = (data["0/Position"][:] + 0.5/1024*200 )*self.Mpc_over_h
I have to say that your explanation was already clear in the issue #62 . I agree with you that my redefinition of the comoving distance is unnecessary and dangerous. And I was completely missing about how handle with the different cosmology or density normalization factors. Thank you again!
I think we have resolved most of these issues now in the main lensing
branch, so I'm gonna go ahead and close this draft PR, but feel free to reopen it Denise if there are things in here you want to merge.
@EiffL I have started to work to some of the task you described in the following issue #62 . I implemented a new lightcone function to export lensplanes in comoving coordinates : https://github.com/DifferentiableUniverseInitiative/flowpm/blob/lensing_check/flowpm/raytracing.py. In the same file I've also implemented the new function for the Born approximation working with plans in comoving distance.
I've created this file https://github.com/DifferentiableUniverseInitiative/flowpm/blob/lensing_check/tests/test_lenstools.py with all the tests we want to implement. It's just a prototype, several things have to be fixed or improved. The first test compares the FlowPM density plane with the Lenstools density plane, right now, the plans have a relative difference higher than the relative tolerance of 0.1 . The second test should validate the interpolation from comoving distance to angular coordinates. To do that , I created a LensTools tracer object with one lens with the same density of FlowPM lens. In the test itself or at the end of this notebook https://github.com/DifferentiableUniverseInitiative/flowpm/blob/lensing_check/notebooks/lightcone_comovingdistance.ipynb you can see how I made this comparison. If you look at the density maps you can see that they are different, I think there is a misconception in how I built the Tracer object.