Python 184 - Tidying

Python Asteroids on GitHub

I have no particular goal in mind this morning — yet. We’ll look at our recent work and then beyond it. My initial concern: Aliasing.

Aliasing

Readers may recall that this test passes:

    def test_show_vector_aliasing(self):
        original = Vector2(100, 100)
        copied = original
        copied += Vector2(10, 20)
        assert copied == Vector2(110, 120)
        assert original == copied  # !!!! aliasing.

Since assignment does not make a new instance, original and copied are references to the same object. The += operation on a Vector2 changes the object in place (mutates it, we like to say), so original changes as well.

This is concerning, because I actually encountered a problem in the aiming logic when I was incrementing a vector that was a reference to another, not a copy, and they both got modified when that was emphatically not what I needed. So I want to try to understand just how Python works in this regard, and to be sure that my code doesn’t do anything problematical.

In my view, the times when one wants aliasing are far fewer than the times when one does not, and when it happens and we’re not aware of it, some very strange things can happen.

I am not clear just exactly what to call Python’s argument passing rules. Let’s write a new test. What I want to know is whether, when I pass an object into a class, and mutate the object inside the class, is the outer object also mutated.

I write a small test class:

class MutatorTest:
    def __init__(self, vector):
        self.vector = vector
        
    def mutate(self, adjustment):
        self.vector += adjustment

Now let’s write a test.

class TestPython:
    def test_aliasing_of_parameters(self):
        original = Vector2(100, 100)
        mutator = MutatorTest(original)
        mutator.mutate(Vector2(10, 20))
        assert mutator.vector == Vector2(110, 120)
        assert original == mutator.vector  # aliasing!!!

This test passes. original does get changed.

This induces me to look carefully at everywhere that I use +=. Let’s first do another test.

    def test_aliasing_of_int(self):
        original = 15
        mutator = MutatorTest(original)
        mutator.mutate(5)
        assert mutator.vector == 20
        assert original == 15

This also passes! The original does not get changed. Same object, same code, different behavior. Fascinating.

Let’s try a list.

    def test_aliasing_of_list(self):
        original = [1, 2, 3]
        mutator = MutatorTest(original)
        mutator.mutate("a")
        assert mutator.vector == [1, 2, 3, "a"]
        assert original == mutator.vector  # aliasing!!

That’s not weird at all, is it?

Whatcha gonna do, Ron?

Well, what I’m gonna do is I’m gonna search out all my += and think about them. Doing so, I get good news. Aside from various tests checking mutability and aliasing, all my uses of += are using it on numbers. So I think we’re OK.

I should mention that I have read quite a few pages on Python’s calling sequence rules, and have frankly found them neither clear nor consistent. I have not found anything that is both. If you have the facts, please pass them on to me, ronjeffries at mastodon dot social or, for a little while longer, ronjeffries at twitter dot com.

Let’s commit these tests. Commit: various testing of mutability of objects and parameters.

OK, that’s good. I think we are safe from at least this most obvious aliasing issue.

Oh. I want to review the targeting logic one more time before we move away, but that reminds me that I wanted to talk about that strange parameter in FiringSolution.

The ShotOptimizer has two primary methods, random_solution and targeted_solution. Each of those returns a FiringSolution, which looks like this:

class FiringSolution:
    def __init__(self, target_position, shooter_position, safe_distance, speed_adjustment):
        direction_to_target = (target_position - shooter_position).normalize()
        safety_offset = direction_to_target * safe_distance
        self.velocity = direction_to_target * u.MISSILE_SPEED * speed_adjustment
        self.start = shooter_position + safety_offset

    def saucer_missile(self):
        return Missile("saucer", self.start, self.velocity)

What is that final parameter, speed_adjustment? Well, it’s hysterical. No, wait, historical. In earlier times, with different targeting, in order to have a targeted missile hit the ship, I needed to slow down the missile a bit, because the missile doesn’t start at the saucer, it starts a bit away from it, so as to not accidentally blow up the saucer. Our new scheme adjusts for the safety offset directly, as we’ll see below, and so we are always passing 1 to this parameter. It is no longer needed, extra, redundant, and unnecessary.

Let’s remove it. Change Signature:

class FiringSolution:
    def __init__(self, target_position, shooter_position, safe_distance):
        direction_to_target = (target_position - shooter_position).normalize()
        safety_offset = direction_to_target * safe_distance
        self.velocity = direction_to_target * u.MISSILE_SPEED
        self.start = shooter_position + safety_offset

Tests are green. But this thing isn’t tested much. On the other hand, the changes we made couldn’t possibly break anything¹ … commit: remove useless parameter.

Let’s review the whole targeting scheme now, perhaps for the final time for a while.

Here’s the new AimImprover:

class AimImprover:
    def __init__(self, best_target_position, target_velocity, shooter_position, missile_speed, safe_distance):
        self.original_target_position = best_target_position
        self.target_velocity = target_velocity
        self.shooter_position = shooter_position
        self.missile_speed = missile_speed
        self.safe_distance = safe_distance

    def improved_aiming_point(self, initial_aiming_point):
        missile_travel_distance = self.shooter_position.distance_to(initial_aiming_point) - self.safe_distance
        missile_travel_time = missile_travel_distance / self.missile_speed
        target_motion_vector = missile_travel_time * self.target_velocity
        anticipated_ship_position = self.original_target_position + target_motion_vector
        return anticipated_ship_position

This code tells the story of aim improvement as well as I have been able to tell it, so far. Let’s read the improved_aiming_point method in what I’ll call English.

Improving Aiming Point

We have an initial_aiming_point, our current guess at where the target will be when our missile gets there. In use, initial_aiming_point starts at the target’s initial location, and we iteratively improve the aiming point a few times for better results.

The missile_travel_distance to get to the initial_aiming_point point is the distance from the shooter to that point … minus the safe distance, the head start that the missile gets so as not to destroy the shooter.

The missile_travel_time needed to get there is of course that distance divided by the missile’s speed.

During the missile_travel_time, the target will move at its current velocity, some vector amount. Its target_motion_vector will be its velocity times the missile_travel_time.

Therefore, after the missile_travel_time has elapsed, we estimate that the target will be at its original starting position plus that target_motion_vector. We call that anticipated_ship_position.

So we return anticipated_ship_position as the new recommended improved_aiming_point.

Over the course of writing that, I changed two of the variable names in the code you see above, moving away from saying “ship” to consistently saying “target”, and referring to the motion vector as a vector. It had previously just been target_motion.

Well

Well, more consistently. I found out below that I missed one. Easy to do.

OK, this is pretty decent. We could extract two methods here. Let’s do it just to look at the result and see if we prefer it.

    def improved_aiming_point(self, initial_aiming_point):
        missile_travel_time = self.missile_travel_time_to(initial_aiming_point)
        anticipated_target_position = self.target_position_at_time(missile_travel_time)
        return anticipated_target_position

    def missile_travel_time_to(self, initial_aiming_point):
        missile_travel_distance = self.shooter_position.distance_to(initial_aiming_point) - self.safe_distance
        missile_travel_time = missile_travel_distance / self.missile_speed
        return missile_travel_time

    def target_position_at_time(self, missile_travel_time):
        target_motion_vector = missile_travel_time * self.target_velocity
        anticipated_target_position = self.original_target_position + target_motion_vector
        return anticipated_target_position

I noticed that I still used the word “ship” in the previous version, fixed it here.

Let’s try reading that in “English”.

Improving Aiming Point

We have an initial_aiming_point, our current guess at where the target will be when our missile gets there. In use, initial_aiming_point starts at the target’s initial location, and we iteratively improve the aiming point a few times for better results.

We get the missile_travel_time to the initial_aiming_point.

Using that, we get the anticipated_ship_position at that future time. We return that position as a better point at which to aim.

Details: Travel Time

To get the missile_travel_time_to our initial_aiming_point, we calculate the missile_travel_distance as the distance from the shooter to the initial_aiming_point, minus the safe_distance, the offset that the missile gets so as not to destroy the shooter.

Then the missile_travel_time is just that distance, divided by the missile_speed.

Details: Anticipated Position

We need to know how far the target will move during the missile_travel_time. We get the target_motion_vector by multiplying the target_velocity by the given missile_travel_time.

Then the anticipated_target_position is just the original_target_position plus the target_motion_vector.

Better? I think so. I think I’ll keep it. I prefer the extra explanation that the method names provide, since there are actually two phases to the calculation, getting the time and then getting the position. Commit: refactor for clarity.

YMMV, but I prefer tiny objects and tiny methods. My house, my rules.

Are we there yet?

When some programmer comes across this object in the future, and for some reason wants to understand it, we want them to read the code and quickly see what’s going on. We want them to think:

OK, I see. We just get the missile travel time and see where the target will be at that time and aim there.

If they want details, we want them to think:

OK, the travel time is just the distance over speed. There’s a safe distance subtracted from the distance. Huh? What is that?

We don’t really want that last bit but I don’t know how to make it better. Perhaps something will come to mind. Anyway if they want the other details, we want them to think:

Right, the target position when the missile gets there will be its starting position plus its travel vector, which will be its velocity times the time.

I get it, nothing to it.

So are we there yet? Maybe not, but we’ve given that future programmer our best ideas so far.

Lesson

For me, one lesson is that just like our verbal explanations of things, the code is never perfectly good at communicating. When we tell a story, we refine it over time until its the best story we can tell. When we write code, we can refine it over time, until it tells its own story as well as it can.

That’s an iterative process. We can stop at any time, and it appears to me that we can continue to improve things at any time later.

Lesson

Another lesson, which you can see in the series of articles up to here, is that we can try many ways of helping the code tell its story. Some of them will be better and some will not. I personally find that actually making the change I’m considering lets me assess “better or not” more readily than I can do so by speculating. I need to see the new code, not just imagine it.

Caveat

Now of course we cannot over the course of a few working days, polish some small object again and again and again, as I have done here. We’d never make any progress if we did that. But we can, over the course of every working day, improve the code we’re working with just a little bit.

Since we’re working on it, the odds are good that we’ll work on it again. It just seems to work out that way. So improving it a bit will help whoever passes this way next time, and over time, small improvements wherever we are tend to keep the code alive and easier to change.

And, as we have been told, we are in the business of changing code.

Enough

I had thought we might look at ShotOptimizer this morning, but we have found enough meat here in our tests for aliasing, and in this small one-method object, AimImprover. We’ll call this session to a close.

Looking back over recent history, we’ve moved from targeting as part of Gunner, to a separate ShotOptimizer object, then adding in a FiringSolution object that can create a missile given parameters, and then finally added the AimImprover that manages the iterative creation of the information we need.

I prefer this kind of thing, what we sometimes call “separation of concerns”. Whether this particular separation is just right or only “nearly good”, I prefer it to having all the disparate concerns inside Gunner. Again, YMMV. I’d suggest trying small objects, getting used to them … and then deciding in individual cases what will be preferable for you.

But you do you. It’s your programming, not mine.

See you next time!