#134 - shear conventions

[rmandelb]

This pull request is for some agreed-upon updates to our shearing conventions. It includes:

• on the C++ side, Shear objects are now initialized using g1/g2 (reduced shear) rather than e1/e2-type distortions; likewise the C++ applyShear works in terms of g1/g2.
• on the python side, there is a Shear method that can be initialized in many different ways, using reduced shear / distortion / conformal shear expressed either as components or as a total normalization and a position angle. There are unit tests to make sure these different initializations are all working properly. The applyShear/createSheared methods can take either a python Shear as an argument, or can take arguments to initialize its own Shear.
• there are some simplifications in how Ellipses are initialized, including the definition of a python Ellipse. Its behavior is not quite parallel to Shear, btu Ellipse is also not used as much, so I think that's okay for now. We can use this one for a while and then I can change it if we hate it.
• the applyDistortion/createDistorted methods are now applyTransformation/createTransformed. Also, there are more unit tests that use those methods.
• config is only minimally updated to use the proper methods but without adding more ways to initialize shears, per Jim's suggestion that I wait for his ongoing method to update config
• added getG1(), getG2() methods

Limitations because I ran out of time before i have to move:

• I realized that the HSMShapeData structure contains a C++ shear, whereas we might want it to have a python Shear. The difference is that if it's a C++ shear you have to do e.g.,

res = galsim.HSMShapeData()
g1 = res.observed_shape.getG1()

whereas if it's a python shear you could do

g1 = res.observed_shape.g1

which is slightly simpler. I didn't think it was a big deal and I'm out of time for a while so I didn't bother redefining the structure in python, but I could do it later if people want it.

• I wasn't sure what is our convention for doxygen documentation of possible args/kwargs that could be used in the Shear/Ellipse constructors. It didn't seem quite right to put them as params in the usual way. So right now the dox are slightly inadequate (with some useful examples but no params). Once I get some suggestions I can improve this.
• ShootInterpolated.py cannot be run, but that's an issue on master as well rather than something new, so I am putting in this pull request and hoping it will get fixed elsewhere rather than on this branch.

Because of the move, I'll be semi-responsive for the next week, but should be able to respond to concerns periodically so we can get this merged in within ~1 week.

[rmjarvis]

Did you have any thoughts on the question I raised regarding proper documentation of possible args/kwargs for all the ways to make a Shear or Ellipse?

I like the current style you use to document the ways of constructing Shear and Ellipse. I think that's going to be clearer than trying to find a way to make doxygen params work well. The only suggestion I would make is to try to make sure the example list is complete. Not necessarily every possible combination in the code block, but probably add (eta1,eta2), (g,beta), and (e,beta) to the explicit list for Shear.

I'm not sure what would be the sufficient list for Ellipse, but probably add a few more like (mu, shift, s), (shift, mu) to point out that they can be in any order, and also some that use the shear arguments rather than a pre-constructed shear.

[rmandelb]

Thanks for the feedback about the dox. I'll fix it up tomorrow/Tues - have to go collapse now after a long day of packing.

[rmandelb]

Time for a break in moving, so I updated the dox. At this point, Mike, I think I've addressed all of your comments, but we should give it a few days for others to chime in.

On Jun 17, 2012, at 10:12 PM, Mike Jarvis wrote:

Mike, thanks for your incredibly thorough comments. I will do these as soon as I can and will let you know when they are pushed. Did you have any thoughts on the question I raised regarding proper documentation of possible args/kwargs for all the ways to make a Shear or Ellipse?

I like the current style you use to document the ways of constructing Shear and Ellipse. I think that's going to be clearer than trying to find a way to make doxygen params work well. The only suggestion I would make is to try to make sure the example list is complete. Not necessarily every possible combination in the code block, but probably add (eta1,eta2), (g,beta), and (e,beta) to the explicit list for Shear.

I'm not sure what would be the sufficient list for Ellipse, but probably add a few more like (mu, shift, s), (shift, mu) to point out that they can be in any order, and also some that use the shear arguments rather than a pre-constructed shear.

---

Reply to this email directly or view it on GitHub:
#193 (comment)

---

Rachel Mandelbaum
http://www.astro.princeton.edu/~rmandelb
rmandelb@astro.princeton.edu

[rmjarvis]

At this point, Mike, I think I've addressed all of your comments, but we should give it a few days for others to chime in.

I agree. I added another line to the doc to make explicit that my erroneous assumption about beta isn't correct. I also merged in the current master, so that's up to date now as well. I think we can merge this in tomorrow unless anyone has any objections.

[rmandelb]

Thanks for the merge and update, and I see that #21 made its way in. I just did another minor update for consistency of notation etc. I'm happy to merge this into master tomorrow night (I think we should give one more working day for people to respond).

On Jun 18, 2012, at 6:26 PM, Mike Jarvis wrote:

At this point, Mike, I think I've addressed all of your comments, but we should give it a few days for others to chime in.

I agree. I added another line to the doc to make explicit that my erroneous assumption about beta isn't correct. I also merged in the current master, so that's up to date now as well. I think we can merge this is tomorrow unless anyone has any objections.

---

Reply to this email directly or view it on GitHub:
#193 (comment)

---

Rachel Mandelbaum
http://www.astro.princeton.edu/~rmandelb
rmandelb@astro.princeton.edu

[rmandelb]

@barnabytprowe @gbernstein @TallJimbo - I see various comments from you on commits, but it wasn't clear if (a) you were going to look over the rest of the pull request but didn't have time, (b) you looked the rest over and have no comments, or (c) you don't plan to look it over and want to leave the details to me and Mike. If (a), please let us know before tonight so we can give you more time, otherwise one of us might go ahead and merge it into master...

Mike: at some point this went back into a "cannot be automatically merged" state. Since I have a history of screwing up resolution of complicated merges and am completely exhausted / not thinking clearly from this move, would you be willing to deal with those conflicts? TIA...

[barnabytprowe]

I need to give one more broad look, but that should be done by this p.m. I can also handle the merge if you'd like?

[rmandelb]

Thanks for looking it over. As for the merge, you and Mike can work that out amongst yourselves...

On Jun 19, 2012, at 2:44 PM, Barnaby Rowe wrote:

I need to give one more broad look, but that should be done by this p.m. I can also handle the merge if you'd like?

---

Reply to this email directly or view it on GitHub:
#193 (comment)

---

Rachel Mandelbaum
http://www.astro.princeton.edu/~rmandelb
rmandelb@astro.princeton.edu

[TallJimbo]

I don't have any plans to look over this in any greater detail.

[gbernstein]

Niether do I.

On Jun 19, 2012, at 3:01 PM, Jim Bosch wrote:

I don't have any plans to look over this in any greater detail.

---

Reply to this email directly or view it on GitHub:
#193 (comment)

[rmandelb]

Okay, so Barney can merge when he's done, or ask Mike to (unless significant issues arise that we have to solve before merging).

[barnabytprowe]

Mike already merged, and I just looked over the pull request on final time. Looks great, I'm going to merge this into master. Thanks all!