Starlink / starlink

Starlink Software Collection
162 stars 53 forks source link

alternate multithreading map rebinning scheme #19

Closed gmarsden closed 10 years ago

gmarsden commented 10 years ago

Change the way smf_rebinmap1.c splits the jobs between threads.

Old version:

New version:

Regression tests show that the maps change by roughly one part in 10^6 (in terms of map SNR). The rebinning routine runs roughly 4 times faster with this new scheme, and reduces the overall makemap runtime by about 10%.

UPDATE 2013-10-28

I realized that I had run these tests with smurf compiled with debugging symbols. When I remcompile without them, the improvement in runtime is less dramatic. The rebinning routine is now about 2 times faster, with overall runtime decreased by about 5%.

timj commented 10 years ago

You might want to squash those last three commits into one so that we get the proper indenting in a single commit.

gmarsden commented 10 years ago

Sorry, that was meant to help readability, but didn't because of the tabs I forgot to convert. I think I know how to squash the commits, but I'm not sure how to update my public fork. Can you tell me how to do that?

timj commented 10 years ago

Generally, it's easier if you create a feature branch off of master when you do your work. Then you can continually rebase the branch as you resync master. Also, on a feature branch you can use --force to overwrite your changes to your branch on github without things getting confused (you can't rewrite anything on master). I think it might be the case that rewriting a branch attached to a pull request causes the pull request to update (but I'm not sure if that works on master).

gmarsden commented 10 years ago

OK, I've squashed all the edits to smf_rebinmap1.c into one commit. I also added in a test to make sure the pixel pointer is in range -- this showed up as an error when I tested the VAL__BADD version. It has the fortunate side-effect of realigning a bunch of unchanged code. The diff should now be easier to view.