search

davidwhogg / LensTractor

finding strong gravitationally lensed quasars in ground-based data
GNU General Public License v2.0
5 stars 4 forks source link

SDSS PhotoCal #26

Closed drphilmarshall closed 10 years ago

drphilmarshall commented 10 years ago

Re-posting as a new issue, to keep things well-compartmentalised. See also discussion in #25

@dstndstn, can you help us get the photocal right for SDSS images, please? dm.py and sdss.py shows the choices being made... its not pretty at the moment, I blithely assume a constant zpt for all SDSS images, and use a simple homegrown PS1 photocal...

Thanks!

aagnello commented 10 years ago

It may be a bit too brutal... I had tried to adapt the header-based initialisations, I've added them now in sdss.py, they're commented out so they should not conflict with the ones implemented by Phil. It's now pushed onto origin refactor. If someone with the required experience (i.e. @dstndstn) could make it work, that'd be of much help.

dstndstn commented 10 years ago

I don't understand the problem. The tractor.sdss get_tractor_image function is extremely well tested. Could you please post an example script that demonstrates the problem you are seeing, with complete command-line.

​thanks, --dustin

aagnello commented 10 years ago

We are not using get_tractor_image, because we'd need to feed fits files directly from the command line. Phil's point was not on possible problems with the tractor, which I'm sure is very well tested! I guess he'd like you to have a look into dm.py and/or sdss.py (inside LensTractor/lenstractor) and tell us if we're doing something evidently wrong with the header-based initialisations.

aagnello commented 10 years ago

(in particular counts-to-mags)

drphilmarshall commented 10 years ago

We'll look at tractor.sdss get_tractor_image and try and use that, in place of what I currently have. Thanks for the pointer, @dstndstn! I'll report back to this thread when I have tried it out. I think fixing this will solve other problems. Adri: let us know if you want me to pull down changes you make on the photocal too, you might get to this before me. Don't forget to tell me which branch to look at...

On Wed, Apr 23, 2014 at 1:25 PM, Adriano Agnello notifications@github.comwrote:

(in particular counts-to-mags)

— Reply to this email directly or view it on GitHubhttps://github.com/davidwhogg/LensTractor/issues/26#issuecomment-41210131 .

aagnello commented 10 years ago

@drphilmarshall I haven't made huge changes, just marginal adjustments to have the code produce something sensible on the sdss examples too. Something's not quite right yet though, I'll keep you posted. I'm working on refactor.

dstndstn commented 10 years ago

-recently I prefer to just use linear brightness units -- nanomaggies directly -- rather than creating Mag objects and having the PhotoCal object convert to flux. To do this, use a LinerPhotoCal(1.) and NanoMaggies() brightness objects.

-it looks like PS1MagsPhotoCal (which is also what SDSS_photocal is returning) is just a copy of tractor.MagsPhotoCal. In general you should avoid doing that -- subclass and override just what you need, don't copy code wholesale. The Tractor codebase changes a fair bit, so you don't want to have an old snapshot of code.

cheers, --dustin

drphilmarshall commented 10 years ago

Thanks Dustin: I'll try out the nanomaggies. You can interpret my copy/paste work as a sign of where my python skill level is! I'll try doing things via inheritance instead, I learned a bit more about this yesterday with Hogg. I think I'll also learn a lot by trying to use the SDSS get_image function.

On Thu, Apr 24, 2014 at 4:49 AM, Dustin Lang notifications@github.comwrote:

-recently I prefer to just use linear brightness units -- nanomaggies directly -- rather than creating Mag objects and having the PhotoCal object convert to flux. To do this, use a LinerPhotoCal(1.) and NanoMaggies() brightness objects.

-it looks like PS1MagsPhotoCal (which is also what SDSS_photocal is returning) is just a copy of tractor.MagsPhotoCal. In general you should avoid doing that -- subclass and override just what you need, don't copy code wholesale. The Tractor codebase changes a fair bit, so you don't want to have an old snapshot of code.

cheers, --dustin

— Reply to this email directly or view it on GitHubhttps://github.com/davidwhogg/LensTractor/issues/26#issuecomment-41270839 .

aagnello commented 10 years ago

Even though we haven't been as elegant as possible for this issue, we now seem to have a sensible calibration and initialization of the model fluxes, so I'll close this ticket.