shears (and other compound parameters) in configs

[TallJimbo]

The Shear class is a little problematic for future configs (#148) or the new approach we're taking with the GSObject classes (#195), because in both of those contexts we need a method on the GSObject to fire when a parameter is changed. That works fine for scalars:

profile = Moffat()
profile.beta = 1.2    # we can make this invoke a __set__ method that can call something on the GSObject, so it's fine
profile.beta += 2.3    # this also invokes __set__, at least for scalar numbers (see #219 for Angles)

And it works fine if we set an entire Shear object at once:

profile.shear = Shear(0.1, 0.0)   # also invokes __set__

But it doesn't work if we set the shear through its own mutators:

profile.shear.setE1E2(0.1, 0.0)  # doesn't invoke __set__; it doesn't even have access to the profile object

The same trouble appears if we use Position, Bounds, or Ellipse objects as GSObject parameters, though I don't think we actually do that anywhere now.

There are a couple of options here, none of them totally satisfactory:

• We could make Shear immutable in Python. Then the last example would become an exception, rather than something that could silently put an object in an inconsistent state. Rachel says we aren't making use of its mutability in Python right now.
• We could hide the actual shear object itself, and have forwarding setters on the thing-that-has-a-Shear:

profile._shear     # where the Shear instance really is; if the user sets it, it's their fault things don't work
profile.setE1E2(0.1, 0.0)    # works fine
profile.e1         # probably want this (and its compatriots) on the thing-that-has-a-shear too for get/set symmetry 
profile.getShear()   # getShear() would return a deep copy of the internal shear, so mutating it wouldn't affect our internals

• We could add callback functionality to the Shear class itself, or make a derived class that has callbacks for when things are set. This would make everything work, but it makes the Shear class and/or adaptor pretty ugly. And there's no way we can control this problem:

s = profile.shear    # s is just another reference to profile.shear
s.setE1E2(0.1, 0.0)   # profile.shear still gets updated, which may be very counterintuitive if s has been passed around a bit

I have a slight preference for the "make Shear immutable" approach - it disallows some convenient syntax, but it doesn't allow anything potentially confusing or quietly dangerous. But I'd like to solicit other opinions on this.

[rmandelb]

To clarify what I meant by the fact that we're not using Shear mutability in python:
In C++ the methods like setE1E2 are crucial because C++ Shears can only be initialized with g1g2. This means that if you want to make a Shear based on anything else (e1/e2 etc.) you have to initialize an empty one and then use the setE1E2 and other such methods. In python, the Shear object that we've made has the setE1E2 and other such methods (based on wrapped methods from C++) but since we can initialize python Shears directly using e1/e2 or various other ways, the set methods are less important.

For Shear I think that making it immutable is clearly the way to go (I'm happy to do so, and could do it quite quickly since I did so much mucking around with it recently). Likewise for Ellipse I think that making it immutable is fine. Other things that modify GSObjects are:

• magnification/dilation, which is fine since it's by a float which is not compound
• rotation by an Angle, which seems like it could be problematic?
• shifting is currently done by dx/dy as floats rather than by a Position, so I think that's fine

[rmjarvis]

It occurs to me with this discussion that putting the shear, magnification, rotation and shift into the GSObjects directly, rather than using the current scheme of applyShear, etc. will require a significant rewrite of these apply methods. e.g. when you do applyDilation, you will have to update all 4 numbers, not just the dilation number. Most of the others will be similar.

I'm also concerned about the user having direct access to these variables: profile.shear = s is problematic if there was already a rotation or shift, since the user may have a different implicit order of operations in mind that what we implement. (Does the shear happen before or after the rotation?) All of these values are like that, since most of them don't commute with each other. (Dilation with either shear or rotation may be the only pairs to commute I think.)

So I think we should probably restrict access to these values to be only via the apply methods even if the values are stored in the GSObject class directly to be applied later. They simply don't work like normal variables that you would want to set and get.

With the old config stuff, we dealt with this by having a very specific order of operations:

ellip
rotate
shear
shift

We can either maintain that definition or have the applications be in the order that they are listed in the config file, but whatever we do we need to be very explicit about how it works, since it has a huge potential for confusion on the part of the user if things work differently than they expect.

[TallJimbo]

This is indeed a big concern, and something I think we should figure out before we press forward too much with converting the GSObject classes.

I have another alternative. I haven't worked out all the details, but I think it's promising:

• Each object has a single linear transform attached to it (conceptually a 2x2 matrix, even if we don't store it that way).
• All these parameters are defined as functions of that 2x2 matrix. In fact, I think we'd probably want to store the transform as a set of these parameters that would allow us to reconstruct the matrix.
• The "apply" methods just multiply a new matrix into the linear transform; we'd then decompose the matrix product into the new parameters. Any "set" methods would replace the current values of those parameters, keeping the rest constant.

I think it's very natural for users to want to know e.g. "the ellipticity" or "the size" of a simulated object, regardless of where those values came from, and I think that's also how they'll naturally think of them in input-catalog form. And while there are a lot of different mathematical definitions in use for many of these parameters, tying them all back to a 2x2 transform matrix is the clearest possible way to define what our code does.

Some issues I can think of:

• We'd be squashing any ellipticities and shears together (an object would only have one ellipticity, even though we could represent it a number of ways). I'm not too bothered by this, because I also think it's also confusing to keep them separate, but often users will want to think of intrinsic ellipticity and applied shear as separate parameters.
• We'd have to be careful about our definition of radius; I think both 4th root of determinant and square root of trace are natural. Maybe we'd provide both?
• Things get trickier when the transform isn't a symmetric matrix and the original object isn't circularly symmetric (e.g. for real galaxies). I think in that case we still have a single linear transform for the object we can apply operations to, but maybe we don't try to decompose it into ellipse parameters.

[rmjarvis]

The problem isn't so much in the "getting", but rather the "setting". The natural thing to do is to have both an intrinsic ellipticity of a galaxy, and an applied shear. It's fine to store them as the net action (which won't be a symmetric matrix!), but we don't want to force the user to only set a single shear. They will often want to set these separately. That's the reason we did both ellip and shear in our old config style.

So I think if we want to have these values stored in the GSObject directly, we should keep both ellip and shear separate. Then obj.shear = s has a well-defined meaning, which I think is the most important thing for having a comprehensible config file.

Then when we need an SBProfile, we build it, shear it by ellip, rotate it, shear it by shear, and then finally shift it. (As appropriate for the values that aren't None of course.) We didn't include the dilation, but that probably belongs either before or after shear (it doesn't matter, since those two do commute), since the inherent size is already given by the initial constructor.

Then if we want, we can provide getters that do some calculation to determine the final net shape or size if people request it.

And the apply methods would normally be ok, since you would typically do this to an object that had Nones to start with. But we would need to provide the correct action when everything is not None and then you shear it. (Especially if shift is not None at that point.) So that's a bit hairy. But the normal use case is not.

[TallJimbo]

I'm was not a fan of having a strict order for these things before, at least at the GSObject level; it is natural for making simple lensing tests, but I don't like the idea of hard-coding that into something that is more generically useful. What about shears due to optical distortions, for instance? Or changes in WCS coordinate systems? It seems like it's just not flexible enough, and that's because we're enforcing a distinction where mathematically there is none.

Perhaps we just shouldn't have any setters on the GSObjects, only "appliers"? And a way to control the order different operations are applied through the config? That's the most mathematically straightforward approach, and we could maintain the ability to return the individual operation parameters if we wanted by storing the chain itself.

[rmjarvis]

Perhaps we just shouldn't have any setters on the GSObjects, only "appliers"?

That is definitely my preference for how the GSObjects work. (i.e. how it works now.) I thought that didn't work with your new config style though.

[TallJimbo]

I hadn't really been planning to have them work that way, but it makes sense, and I think it's certainly possible. An I have no objection to defining things as a sequence of applied transforms, as long as we don't restrict that sequence.

We'll have to think a little about what the config syntax should look like. I'd be more okay with having a lensing-specific order there, because we might have a totally different config schema for doing other kinds of simulations. But if we can come up with a nice way to represent a generic sequence of transforms in config, I'd like that even better.

[rmandelb]

Yes, I agree that it would be good if we can specify order of operations in config.

[TallJimbo]

Could we perhaps take this conversation to #195? I think that's the real issue we're talking about (though it may make this one a moot point).

[rmandelb]

Yes, I think it should really go over there; this issue is just a minor off-shoot of the larger config and GSObject issues.

On Jul 20, 2012, at 1:15 PM, Jim Bosch wrote:

Could we perhaps take this conversation to #195? I think that's the real issue we're talking about (though it may make this one a moot point).

---

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

---

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

[barnabytprowe]

This is now being closed in light of work successfully completed and being merged on #148 and #241.