Fix Version/s: None
Sprint:Science Pipelines DM-W15-5
Team:Data Release Production
The HSM shape code has undergone many improvements and bug fixes as part of being included in the GalSim package, and we've recently included those in the HSC fork of meas_extensions_shapeHSM (HSC-129, HSC-1093). We should transfer those changes to the LSST side before tackling
DM-981 (or at least before finishing it).
The story point estimate here is for the work already done on the HSC side (with the usual 50% factor for shared work). The transfer to the LSST side should be essentially no effort. To the extent that EVM cares about this, the credit should go to Paul Price, even though I (Jim Bosch) am doing the transfer.
- is blocked by
DM-981 convert measurement algorithms in meas_extensions_shapeHSM
I wanted to be able to make changes to the GalSim code (and I did make some small changes) without having to fork it as a separate product. I guess I could have forked it on github and built off the fork, but I guess I was charmed by the status quo, which was including the HSM code in the product.
For LSST, I think it makes sense to fork GalSim on the LSST server, cherry-pick the changes I made, and add the files required to build it in the LSST stack.
GalSim has already been packaged for LSST by the Sims group. Any changes could be added as LSST-only patches or pushed upstream, unless they'd cause divergence from the Sims usage, in which case there could be an issue.
The changes in meas_extensions_shapeHSM are ready for review - this is just a bunch of slightly-modified cherry-picks from the HSC side, with those modifications being:
- use new LSST-side names for exceptions
- don't expect GalSim files to be included in the package; instead build against an external GalSim
As the original changes were already code-reviewed on the HSC side, we don't need to re-review them here, so I'm just asking Paul Price to take a look at:
- the difference between HSC master and this, to make sure I haven't gotten anything wrong;
- the new history, to see if there are new opportunities for squashing commits (I've already done this a little for a few that seemed completely obvious).
Unfortunately, this can't currently be built without some modifications I've made to our GalSim distrib that are waiting on a resolution to whether those should be pushed to GitHub or the old gitolite servers.
I also have an action to push two of Paul's changes to GalSim code upstream:
- Replace an assertion with an exception throw, as it appears possible to trigger the exception when running on real data.
- GalSim should install its own header files (for now, I've patched our build scripts to do this manually).
I've just tested the new GalSim build script and .cfg via buildbot, and it seems to be working fine. I'm going to go ahead and merge that to master and tag it, so I can do a eups distrib release that will allow the shapeHSM branch to be built and tested.
Looks like you've got everything in from HSC.
You could drop a bunch of the early commits, from before we switched to GalSim's HSM (2014-10-23) — they record changes to code that we later removed.
Good to merge once you've demonstrated it's working.
Merged to master.
I did not delete any of the earlier commits, as I felt it was useful to keep the more recent commits a little more similar to the HSC ones - without the older ones, the changesets look very different in some cases.
I also haven't waited for the GalSim eups distrib release - I'm basically blocked by lack of permissions and SQuaRE availability on that. There is a GalSim version on lsst-dev (1.2.lsst1) that can be used to build against, so at present this package can only built there (or with a bit of manual work on GalSim). Since this package was bitrotted and failing tests when I found it, I figure it's at least an improvement to be able to build on one system rather than none.
Paul Price, when you implemented this on the HSC side, did you consider just making GalSim a dependency and building against that? I'm not sure I want to merge all the filtered GalSim git history onto the LSST repo, where I think we're trying to keep the git history a little cleaner. If you just didn't do it because you didn't want to create a eups distrib product for GalSim, I think I might go that route, because I think the sims team has already done that for the LSST side.