search

annayqho / TheCannon

a data-driven method for determining stellar parameters and abundances from stellar spectra
MIT License
39 stars 16 forks source link

code is not safe for ivar=0 values #33

Closed davidwhogg closed 9 years ago

davidwhogg commented 9 years ago

The code does lots of 1/ivar moves which will throw errors or warnings if ivar=0.

davidwhogg commented 9 years ago

can you show me the change that got rid of the 1/ivar moves? Or are you closing as "wontfix"? Because I don't know a lot of ways to do this correctly, and I know lots of ways to do it wrong!

annayqho commented 9 years ago

In one case I used a mask: see line 88, cannon2_infer_labels.py (in Version1.3) and in another case I just used the error array since I use it for the curve_fit anyway: see line 139, apogeedata.py (in Version1.3)

davidwhogg commented 9 years ago

whoops -- I think that code (in infer_labels) is wrong -- recall that if ivar = 0, you want the scattered version also = 0! Note that

1 / ((1/a) + b) = 1 / (1/a + (ab)/a) = a / (1 + ab)

Does that solve your problems?

The current code is wrong, I think.

annayqho commented 9 years ago

Oops, thanks! A question about masking, since I'm not sure what the best thing to do is. I have an ivar array with zeros in it, and I want to use err = 1/np.sqrt(ivar) for (for example) use in curve_fit. Is it poor form to assign some LARGE = 100000. value to all of the ivar==0 indices? And if so, what is the best way to get around that?

davidwhogg commented 9 years ago

Yes that is poor form! But what is done in the Ness Cannon?

annayqho commented 9 years ago

I think that there are some LARGE values used (see for example lines 71, 100 in fitspectra.py) but it's not necessary to do 1/sqrt(ivar) because this version passes error arrays around, whereas mine only passes ivar arrays around.

annayqho commented 9 years ago

Now ivar == 0 only before continuum normalization takes place. After that, it has the value SMALL, where LARGE = 100. and SMALL = 1. / LARGE

davidwhogg commented 9 years ago

Just make sure that whenever you set ivar to SMALL you also set flux to 1.0. This is important, because a HUGE flux value would ruin our whole day, even with a small ivar.