search

LSSTDESC / augur

DESC forecasting and inference validation tool
MIT License
4 stars 2 forks source link

Fixes and config files to reproduce SRD #13

Closed fjaviersanchez closed 3 years ago

fjaviersanchez commented 3 years ago

This PR addresses #10 and #11, building upon #12

c-d-leonard commented 3 years ago

Is this ready to merge? me and @nikosarcevic are playing with augur / firecrown for some forecasts and it would be great to have these updates to generate.py in master. I could review but I'm not that familar with augur - @slosar ?

fjaviersanchez commented 3 years ago

Thanks @c-d-leonard! I think this is ready to go, but I'd appreciate a look before merging.

slosar commented 3 years ago

Is this ready to merge? me and @nikosarcevic are playing with augur / firecrown for some forecasts and it would be great to have these updates to generate.py in master. I could review but I'm not that familar with augur - @slosar ?

@c-d-leonard Danielle, if you could review as well that would be great. I had a quick look, but it is 10PM here and time to go to bed. But I'm super stoked to see this shit moving forward.

fjaviersanchez commented 3 years ago

Thanks for taking a look @c-d-leonard and for offering to help! Yes, I also had some back and forth with the bias mismatch that you point out. I think that Tim's magic numbers correspond to 1.33/growth (which makes me think that there's a sigma8 factor missing somewhere). So your suggestion would be to just use 0.95/growth in the config, right? I think I can take care of these updates today, but if you want to take a stab at something in particular, please let me know! Thanks again!

slosar commented 3 years ago

So the bias could be 0.95/g or 1.05/g not sure, but 1.33/g is def too much (making numbers look better than they should be). What does the SRD document actually say?

c-d-leonard commented 3 years ago

The SRD says b(z) = 0.95 / G(z) for Y10 and b(z) = 1.05 / G(z) for Y1 (section D1.1, page 47). I also just checked the slack history of my chat with Tim about this and he said that the SRD values used the "unnormalised growth" which I took to mean there was an error in their calculation where the growth just didn't get normalised at all - we could probably reverse-engineer what b(z) = Num / G(z) relation that is equivalent to if we want to.

c-d-leonard commented 3 years ago

I'm happy with the way it is currently to merge in - is there anything else you want changed before merging @slosar? I feel like the issue of what are the bias values is kind of a separate question.

slosar commented 3 years ago

If it is good with Danielle it is good with me. I'll let you guys merge. On bias stuff: I've spoken to Rachel and it is known. So one of our first exercises should be to compare SRD with wrong biases vs SRD with the right biases.

slosar commented 3 years ago

I'll let Javi have to Joy of pressing merge, it always give me kicks.