LSST-nonproject / sims_maf_contrib

Contributed code for MAF (sims_maf)
18 stars 46 forks source link

Time delay notebook upgrade #42

Closed drphilmarshall closed 7 years ago

drphilmarshall commented 8 years ago

I am extending the time delay accuracy notebook to carry out the investigation I was supposed to do for the white paper, comparing baseline cadence to kraken_1043 (no visit pairs). I am nearly there, just need to write out a latex table of diagnostic and figure or merit results. I'll let you know when it's ready to merge!

In the meantime my laptop is laboring a bit. Should I be doing some garbage collection to free up memory, @yoachim ? You can see what my program is here. Thanks!

yoachim commented 8 years ago

hmm, nothing jumps out at me as an obvious memory hog. Looks like you added some more keys to what the metric returns--I'd check to make sure none of those are large arrays, that would make things blow up. And you can try turning the nside way down to see if that helps (and define nside once near the top).

rhiannonlynne commented 8 years ago

And maybe check version of maf to be sure you have the kdtree fixes??

On Wed, Aug 24, 2016 at 9:28 AM Peter Yoachim notifications@github.com wrote:

hmm, nothing jumps out at me as an obvious memory hog. Looks like you added some more keys to what the metric returns--I'd check to make sure none of those are large arrays, that would make things blow up. And you can try turning the nside way down to see if that helps (and define nside once near the top).

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/LSST-nonproject/sims_maf_contrib/pull/42#issuecomment-242127365, or mute the thread https://github.com/notifications/unsubscribe-auth/ABxQ3-sXWz2PgbbfTsdhsdpb8adstEOEks5qjHFIgaJpZM4JrMaR .

yoachim commented 8 years ago

OK, I was able to run it in a reasonable amount of time and it didn't look like it went over a gig of memory.

side note--why fork rather than just branch off the original repo?

drphilmarshall commented 8 years ago

Thanks both! I am working in a fork because a) I don't think I have write access to the repo, b) it is standard GitHub practice, no? I'm quite happy not having write access BTW, I like the fork and PR workflow.

yoachim commented 8 years ago

I think it is the standard githubby way to do things. It makes sense for stand-alone packages, but since we have eups it's a little more work. Normally, if I wanted to test some code, I would just clone->build->run. But now I have to clone->declare->setup->run. If you were working on a branch in the main repo, since I have the repo already cloned and setup, I could just checkout->run.

drphilmarshall commented 8 years ago

But you can just add my fork as a remote and then pull from that into a local branch before running, no? I agree its a bit convoluted, especially when it comes to you needing to push back. Having said that, it's nice that my PR will automatically close if you pull from me and then push to base master.

On Thu, Sep 8, 2016 at 9:36 AM, Peter Yoachim notifications@github.com wrote:

I think it is the standard githubby way to do things. It makes sense for stand-alone packages, but since we have eups it's a little more work. Normally, if I wanted to test some code, I would just clone->build->run. But now I have to clone->declare->setup->run. If you were working on a branch in the main repo, since I have the repo already cloned and setup, I could just checkout->run.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/LSST-nonproject/sims_maf_contrib/pull/42#issuecomment-245658393, or mute the thread https://github.com/notifications/unsubscribe-auth/AArY96cvshzLNUdXAraXhA62wftiWIGUks5qoDmrgaJpZM4JrMaR .

drphilmarshall commented 7 years ago

OK @yoachim I think I've finished the lens time delays notebook upgrade. It now makes a latex table which I'm about to check into the white paper :-) I also inadvertently pushed to a time-delay-notebook branch on the base repo, sorry! Make of that what you will :-) I'm happy if you merge straight away but feel free to review. @rhiannonlynne you might be interested in the results in the white paper, we are collaborators on our section after all :-)