LSSTDESC / WeakLensingDeblending

Weak lensing fast simulations and analysis of blended objects.
http://weaklensingdeblending.readthedocs.io/en/latest/index.html
Other
14 stars 13 forks source link

Adding Fisher bias calculation. #9

Closed ismael-mendoza closed 8 years ago

ismael-mendoza commented 8 years ago

Adding bias calculation using the Fisher Formalism to the package will be useful. To calculate the biases it is necessary to first obtain the second partial derivatives of the galaxy with respect to its parameters.

An idea is to calculate the second partials in render.py like the first partials and store all the partials together in the datacubes (set an optional flag because this will potentially make the fits file become very big). Then we would need to add the corresponding functions in analysis.py to get the biases from the second partials. Bias images can also be displayed by adding the corresponding functions in fisher.py

dkirkby commented 8 years ago

This sounds useful. Have you thought about how you want to calculate the 2nd derivatives? I used one-sided finite differences for the 1st derivatives, to reduce the number of different GalSim renderings required. The 2nd derivative could re-use the images already rendered for the first derivative.

dkirkby commented 8 years ago

@ismael2395 I would like to add you to the list of collaborators for this repo, but I suspect you need to join the DarkEnergyScienceCollaboration team first.

drphilmarshall commented 8 years ago

@dkirkby - this project is public, so @ismael2395 can just fork the repo and submit a pull request. This will also put him on the watch list as well, so he'll get all the email notifications.

On Tue, Nov 24, 2015 at 5:32 PM, dkirkby notifications@github.com wrote:

@ismael2395 https://github.com/ismael2395 I would like to add you to the list of collaborators for this repo, but I suspect you need to join the DarkEnergyScienceCollaboration team first.

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/WeakLensingDeblending/issues/9#issuecomment-159457999 .

ismael-mendoza commented 8 years ago

Thanks for the clarification @drphilmarshall , I already forked the repo and will do work on it @dkirkby . That is a great idea @dkirkby ! Taking advantage of the images before would make it faster. Once I have a working implementation of this I will post it in this comment. -- actually in the bottom one.

ismael-mendoza commented 8 years ago

I looked through your code @dkirkby and noticed that you calculate your partial derivatives two-sidedly? (render.py, line244)

        # Calculate partial derivative images, if requested.
        if not no_partials:
            for i,(pname,delta) in enumerate(variations):
                variation_stamp = (galaxy.renderer.draw(**{pname: +delta}).copy() - 
                    galaxy.renderer.draw(**{pname: -delta}))
                datacube[i+1] = variation_stamp.array/(2*delta)

So after some calculations I noticed that with a 6 parameter galaxy, doing one-sided second partials for the galaxies would require only 16 total galsim draws and two-sided second partials would require 41 total draws (taking advantage of the images rendered in both cases before the second partials) I was thinking that for robustness sake it is still better to do two-sided derivatives, in any case a good idea would be to add a command line argument like no_second_partials so the user can choose not to calculate second partials if he/she so desires.

dkirkby commented 8 years ago

Yes, it looks like you are right about the two-sided derivative. It must a different project where I used one-sided partials.

Rather than 'no-second-partials' I would name your option something like 'no-bias', to emphasize what is actually calculated (rather than how). If it turns out that the bias calculations slow down the analysis a lot, you should probably reverse the logic so they are disabled by default and something like 'calculate-bias' turns it on.

Note that the flux partials can probably be calculated analytically, so that should speed things up a bit.

drphilmarshall commented 8 years ago

PS. @dkirkby did you also try inviting Ismael to join a team with write access to this repo? I think you should be able to. You have write access through your membership of the WeakLensing team, but also admin access through the Old Owners team, and its the latter that should work even if the former does not. There is precedent for DESC repos having non DESC members with write access - this conversation reminds me that this is something that the publications committee shoudl write about in their policy draft.

On Tue, Dec 1, 2015 at 4:56 PM, Ismael Salvador Mendoza Serrano < notifications@github.com> wrote:

I looked through your code @dkirkby https://github.com/dkirkby and noticed that you calculate your partial derivatives two-sidedly?

    # Calculate partial derivative images, if requested.
    if not no_partials:
        for i,(pname,delta) in enumerate(variations):
            variation_stamp = (galaxy.renderer.draw(**{pname: +delta}).copy() -
                galaxy.renderer.draw(**{pname: -delta}))
            datacube[i+1] = variation_stamp.array/(2*delta)

So after some calculations I noticed that with a 6 parameter galaxy, doing one-sided second partials for the galaxies would require only 16 total galsim draws and two-sided second partials would require 41 total draws (taking advantage of the images rendered in both cases before the second partials) I was thinking that for robustness sake it is still better to do two-sided derivatives, in any case a good idea would be to add a command line argument like no_second_partials so the user can choose not to calculate second partials if he/she so desires.

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/WeakLensingDeblending/issues/9#issuecomment-161145112 .

dkirkby commented 8 years ago

It looks like only the WeakLensing "Team Maintainers" (@mdschneider, @rmjarvis) can invite someone to join the team, and OldOwners is now a regular team with no associated admin access.

drphilmarshall commented 8 years ago

OK, good to know - thanks David!

On Tue, Dec 1, 2015 at 5:33 PM, dkirkby notifications@github.com wrote:

It looks like only the WeakLensing "Team Maintainers" (@mdschneider https://github.com/mdschneider, @rmjarvis https://github.com/rmjarvis) can invite someone to join the team, and OldOwners is now a regular team with no associated admin access.

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/WeakLensingDeblending/issues/9#issuecomment-161151173 .

rmjarvis commented 8 years ago

It looks like only the WeakLensing "Team Maintainers" (@mdschneider, @rmjarvis) can invite someone to join the team, and OldOwners is now a regular team with no associated admin access.

Did you want me to add him?

Or, personally, I'd be perfectly happy to have more people be maintainers. @drphilmarshall, Any problem with adding David as a maintainer. Probably a few others as well I would imagine. But David at least, since he's probably the most active WLer on the repo.

drphilmarshall commented 8 years ago

Done - @dkirkby is now a Weak Lensing team maintainer.

On Tue, Dec 1, 2015 at 6:40 PM, Mike Jarvis notifications@github.com wrote:

It looks like only the WeakLensing "Team Maintainers" (@mdschneider https://github.com/mdschneider, @rmjarvis https://github.com/rmjarvis) can invite someone to join the team, and OldOwners is now a regular team with no associated admin access.

Did you want me to add him?

Or, personally, I'd be perfectly happy to have more people be maintainers. @drphilmarshall https://github.com/drphilmarshall, Any problem with adding David as a maintainer. Probably a few others as well I would imagine. But David at least, since he's probably the most active WLer on the repo.

— Reply to this email directly or view it on GitHub https://github.com/DarkEnergyScienceCollaboration/WeakLensingDeblending/issues/9#issuecomment-161161074 .

dkirkby commented 8 years ago

Thanks @drphilmarshall. It turns out that I can only add existing DarkEnergyScienceCollaboration members, so Ismael will need to join the DESC github organization first.

drphilmarshall commented 8 years ago

Interesting! I just sent @ismael2395 an invitation to the organization. When he has accepted his invitation, maybe you could try altering his status to "Outside Collaborator"? Interested to know if Team Maintainers can do that. They should be able to...

dkirkby commented 8 years ago

I am now declaring this issue closed - thanks for all the hard work @ismael2395 !

ismael-mendoza commented 8 years ago

Thanks for all your help @dkirkby! I am glad to have contributed to your package