Code style query: multiple assignments in if/elif tree

[Chris Angelico]

Call this a code review request, if you like. I'm wondering how you'd
go about coding something like this.

Imagine you're in a train, and the brakes don't apply instantly. The
definition, in the interests of passenger comfort, is that the first
second of brake application has an acceleration of 0.2 m/s/s, the next
second has 0.425 m/s/s, and thereafter full effect of 0.85 m/s/s. You
have a state variable that says whether the brakes have just been
applied, have already been applied for at least two seconds, or
haven't yet been applied at all. Problem: Work out how far you'll go
before the brakes reach full power, and how fast you'll be going at
that point.

Here's how I currently have the code. The variable names are a tad
long, as this was also part of me teaching my brother Python.

# Already got the brakes fully on
if mode=="Brake2": distance_to_full_braking_power, speed_full_brake =
0.0, curspeed
# The brakes went on one second ago, they're nearly full
elif mode=="Brake1": distance_to_full_braking_power, speed_full_brake
= curspeed - 0.2125, curspeed - 0.425
# Brakes aren't on.
else: distance_to_full_braking_power, speed_full_brake = (curspeed -
0.1) + (curspeed - 0.4125), curspeed - 0.625
# If we hit the brakes now (or already have hit them), we'll go
another d meters and be going at s m/s before reaching full braking
power.

But I don't like the layout. I could change it to a single assignment
with expression-if, but that feels awkward too. How would you lay this
out?

(Note that the "else" case could have any of several modes in it, so I
can't so easily use a dict.)

ChrisA

 

[Marko Rauhamaa]

Call this a code review request, if you like. I'm wondering how you'd
go about coding something like this.

As a simple layout question, I'd do it like this:

========================================================================
if mode == "Brake2":
# Already got the brakes fully on
distance_to_full_braking_power = 0.0
speed_full_brake = curspeed
elif mode == "Brake1":
# The brakes went on one second ago, they're nearly full
distance_to_full_braking_power = curspeed - 0.2125
speed_full_brake = curspeed - 0.425
else:
# Brakes aren't on.
distance_to_full_braking_power = (curspeed - 0.1) + (curspeed - 0.4125)
speed_full_brake = curspeed - 0.625

# If we hit the brakes now (or already have hit them), we'll go another
# d meters and be going at s m/s before reaching full braking power.
========================================================================


Marko

 

[Chris Angelico]

As a simple layout question, I'd do it like this:

========================================================================
if mode == "Brake2":
# Already got the brakes fully on
distance_to_full_braking_power = 0.0
speed_full_brake = curspeed
elif mode == "Brake1":
# The brakes went on one second ago, they're nearly full
distance_to_full_braking_power = curspeed - 0.2125
speed_full_brake = curspeed - 0.425
else:
# Brakes aren't on.
distance_to_full_braking_power = (curspeed - 0.1) + (curspeed - 0.4125)
speed_full_brake = curspeed - 0.625

# If we hit the brakes now (or already have hit them), we'll go another
# d meters and be going at s m/s before reaching full braking power.
========================================================================

No particular advantage over the current version - it doesn't simplify
it any, all you've done is break it across more lines.

(The unpacking may not be ideal; as I said, this was a vehicle for
teaching oddments of Python, so I used multiple assignment partly for
the sake of using it.)

Incidentally, if you want to see the code in context, it's here:

https://github.com/Rosuav/runningtime/blob/master/runningtime.py

ChrisA

 

[Rustom Mody]

No particular advantage over the current version - it doesn't simplify
it any, all you've done is break it across more lines.

Not answering your original question but the above comment

Your version was sufficiently garbled indentation-wise (it may give you sweet
pleasure to know my 'client' is GG) that I gave up on reading it.

Marko's version was clear enough that it seems to match your spec
[Ive not really verified it word for word... Just sayin']

On the whole I prefer multiple assignments.
Maybe in this case use small variable names with
separate(d) explanatory comments??

 

[Chris Angelico]

On the whole I prefer multiple assignments.
Maybe in this case use small variable names with
separate(d) explanatory comments??

Shorter variable names would certainly be the more normal, heh. I let
my brother do that part of the typing, picking names and so on. Better
for teaching to let the victi-- err, the student have the power to
choose.

ChrisA

 

[Ned Batchelder]

Incidentally, if you want to see the code in context, it's here:

https://github.com/Rosuav/runningtime/blob/master/runningtime.py

ChrisA

I know you didn't ask about these aspects, but they jumped out at me:
tabs for indentation instead of spaces, and docstring-style comments in
places that aren't docstrings. These seem like unusual choices.

 

[Chris Angelico]

I know you didn't ask about these aspects, but they jumped out at me: tabs
for indentation instead of spaces, and docstring-style comments in places
that aren't docstrings. These seem like unusual choices.

Tabs instead of spaces? They're plenty common enough, but I am *not*
getting into that debate now. The file's perfectly consistent - aside
from inside strings, all indentation is done with tabs, one per level.

How do you go about doing multi-line comments? I know I've seen other
code using triple-quoted strings for long comments before.

ChrisA

 

[Chris Angelico]

Just use a sequence of one-line comments::

# Lorem ipsum dolor sit amet, consectetur adipiscing elit. Ut a
# sapien tempor, suscipit orci sed, elementum nisl. Suspendisse at
# lacus ut diam dignissim lobortis ac vitae augue.
#
# Phasellus bibendum neque a justo vulputate, quis accumsan quam
# egestas. Etiam aliquet blandit ante sit amet cursus.

A decent code editor (e.g. Emacs, Vim) will allow manipulation of
a sequence of one-line comments in Python's comment style, and allow
treating it as a paragraphs for purposes such as re-wrapping the lines.

I agree with others that triple-quoted strings are best reserved for
string literals (including docstrings), not comments.

Fair enough. I can't remember where (or when!) it was that I learned
triple-quoted strings were appropriately abused as comments, so I've
just done a quick re-layout into hash comments.

ChrisA

 

[Ethan Furman]

Fair enough. I can't remember where (or when!) it was that I learned
triple-quoted strings were appropriately abused as comments, so I've
just done a quick re-layout into hash comments.

Personally, I use the sequence of one-line comments.

But, hey, Guido [1] himself likes the triple-quoted string as comment feature [2], so feel free to use it yourself if
you like.

 

[Steven D'Aprano]

]

Fair enough. I can't remember where (or when!) it was that I learned
triple-quoted strings were appropriately abused as comments, so I've
just done a quick re-layout into hash comments.

Probably here

But note the emphasis on "abused". The only time I would use triple-
quoted strings as comments is if I wanted to quickly comment out a
section of code:

do_this()
do_that()
'''
def do_something_else():
"""Docstring"""
pass
do_something_else()
'''
do_more()
do_less()


sort of thing. (Note the cunning use of ''' instead of """.) But I
wouldn't leave it like that in production code.

It's *tempting* to use """ to mark out a large block of text, and I
wouldn't say that doing so was wrong, but it's a bit different, and
programmers are very like cats: they don't like different.

 

[Steven D'Aprano]

Call this a code review request, if you like. I'm wondering how you'd go
about coding something like this.

I wouldn't. I'd start off by analysing the problem, and putting it into
the simplest format possible, and *then* start writing code if and only
if needed. See below.

The first mistake of computational mathematics is to do the computation
before the mathematics, and the Holy Grail is to avoid the computation
altogether.

Imagine you're in a train, and the brakes don't apply instantly. The
definition, in the interests of passenger comfort,

Ah, you're using comfort in the modern sense, i.e. what people used to
call discomfort

The scenario you describe has (effectively) infinite rate-of-change-of-
acceleration, often called "jerk". (A jerk is a rapid change in
acceleration.) Human comfort is (within reasonable limits) more affected
by jerk than acceleration. The passengers will feel three quite
distinctive jerks, one when the brakes are first applied (which is
probably reasonable), then one at 1s, then again at 2s. That's not
comfortable by any stretch of the imagination.

is that the first
second of brake application has an acceleration of 0.2 m/s/s, the next
second has 0.425 m/s/s, and thereafter full effect of 0.85 m/s/s.

In mathematics, this is called hybrid function, and is usually written
like this:

g(t) for t < 0
f(t) = { 42 for t == 0
h(t) for t > 0

or something similar. (The opening brace { ought to be large enough to
cover all three lines, and there is no closing brace.)

In your case, you have three constant functions.

You
have a state variable that says whether the brakes have just been
applied, have already been applied for at least two seconds, or haven't
yet been applied at all.

I wouldn't model it that way. Especially since you've missed at least one
state I'd model the acceleration as a function of time, otherwise you
have to convert a time into a state. Nevertheless, if you insist, we can
use a state variable instead:

0 for state == "BRAKES NOT APPLIED YET"
accel = { 0.2 for state == "BRAKES ON FOR BETWEEN 0 AND 1 SECOND"
0.425 for STATE == BRAKES ON FOR BETWEEN 1 AND 2 SECOND"
0.85 for state == "BRAKES ON FOR MORE THAN 2 SECONDS"

Implied, but not stated, is that once the train stops, acceleration also
goes to zero -- the train does not start moving backwards.

Haskell has nifty pattern-matching syntax for this that looks quite close
to the mathematical hybrid function syntax, but in Python, we're limited
to explicitly using an if. If I were coding this, and I'm not, I'd wrap
it in a function. One advantage of a state variable rather than a
continuous time function is that we can do this:

def accel(state):
return {NO_BRAKING: 0.0,
LOW_BRAKING: 0.2,
MID_BRAKING: 0.425,
HIGH_BRAKING: 0.85}[state]


which is simple enough to skip using a function in the first place, and
just use the dict lookup directly. But for a more general question you'll
want acceleration as a function of time.

If you prefer if...elif over a dict, I'd still hide it in a function.

Problem: Work out how far you'll go before the
brakes reach full power, and how fast you'll be going at that point.

Given that problem, we actually only care about LOW_BRAKING and
MID_BRAKING. The problem becomes quite simple:

At t=0, the train is travelling at u m/s and the brakes are applied with
acceleration of 0.2m/s^2 for one second, then 0.425m/s^2 for an
additional one second. What is the speed of the train after those two
seconds, and the distance travelled.

This becomes a simple question for the four standard equations of motion:

(1) v = u + at
(2) s = 1/2(u + v)t
(3) s = ut + 1/2(at^2)
(4) v^2 = u^2 + 2as

Only (1) and (3) are needed.


The distance travelled in the first second is:

s1 = u - 0.1 m

and the speed of the train at that time becomes:

v = u - 0.2 m/s

Applying this for the second second gives:

s2 = (u - 0.2) - 0.2125 m

v = (u - 0.2) - 0.425 m/s

and the total distance:

s = s1 + s2


No programming required, just a calculator

Given that solving the entire problem is barely five lines, writing code
to do so is probably unnecessary.

The only thing I haven't done is check that the train isn't travelling so
slowly that it comes to a complete halt within two seconds. I leave that
as an exercise. (Hint: if the final speed is negative, the train came to
a halt.)

Here's how I currently have the code. The variable names are a tad long,
as this was also part of me teaching my brother Python.

Whatever you used to post this, ate the indentation.

# Already got the brakes fully on
if mode=="Brake2": distance_to_full_braking_power, speed_full_brake =
0.0, curspeed
# The brakes went on one second ago, they're nearly full elif
mode=="Brake1": distance_to_full_braking_power, speed_full_brake =
curspeed - 0.2125, curspeed - 0.425 # Brakes aren't on.
else: distance_to_full_braking_power, speed_full_brake = (curspeed -
0.1) + (curspeed - 0.4125), curspeed - 0.625 # If we hit the brakes now
(or already have hit them), we'll go another d meters and be going at s
m/s before reaching full braking power.

Using sequence unpacking just to save a newline is naughty

dist, speed = (curspeed - 0.1) + (curspeed - 0.4125), curspeed - 0.625

is an abuse of syntax, not far removed from:

dist = (curspeed - 0.1) + (curspeed - 0.4125); speed = curspeed - 0.625

It will be much more comprehensible written as two statements.

But I don't like the layout. I could change it to a single assignment
with expression-if, but that feels awkward too. How would you lay this
out?

(Note that the "else" case could have any of several modes in it, so I
can't so easily use a dict.)

It could have, but doesn't. Is your aim to build a general purpose
Newtonian linear motion solver, or to solve this one problem?

 

[Chris Angelico]

I wouldn't. I'd start off by analysing the problem, and putting it into
the simplest format possible, and *then* start writing code if and only
if needed. See below.

The first mistake of computational mathematics is to do the computation
before the mathematics, and the Holy Grail is to avoid the computation
altogether.

Fair enough.

Ah, you're using comfort in the modern sense, i.e. what people used to
call discomfort

The scenario you describe has (effectively) infinite rate-of-change-of-
acceleration, often called "jerk". (A jerk is a rapid change in
acceleration.) Human comfort is (within reasonable limits) more affected
by jerk than acceleration. The passengers will feel three quite
distinctive jerks, one when the brakes are first applied (which is
probably reasonable), then one at 1s, then again at 2s. That's not
comfortable by any stretch of the imagination.

It actually is a smooth increase in deceleration, but I'm operating
the simulator on a 1s period, so it's actually an average across the
first second, and an average across the next second...

.... so really, it's seeking upward from 0 to 0.85 according to some
curve, the details of which I'm not familiar with, but in terms of 1s
average accelerations, it's those figures. Actually even 1s resolution
is more than we need; the overall purpose is to get a calculated time
in minutes, so calculating to sub-second resolution (when, in reality,
this is trying to predict what a human will do) is massive overkill.

I wouldn't model it that way. Especially since you've missed at least one
state I'd model the acceleration as a function of time, otherwise you
have to convert a time into a state. Nevertheless, if you insist, we can
use a state variable instead:

0 for state == "BRAKES NOT APPLIED YET"
accel = { 0.2 for state == "BRAKES ON FOR BETWEEN 0 AND 1 SECOND"
0.425 for STATE == BRAKES ON FOR BETWEEN 1 AND 2 SECOND"
0.85 for state == "BRAKES ON FOR MORE THAN 2 SECONDS"

Implied, but not stated, is that once the train stops, acceleration also
goes to zero -- the train does not start moving backwards.

"Just been applied" is poorly named; that's the state where the brakes
have been applied for 1 second so far. The other state (brakes not
applied yet) could be looking at powering (which follows a similar
curve in the opposite direction) or cruising (which has acceleration
of 0.0 m/s/s).

Haskell has nifty pattern-matching syntax for this that looks quite close
to the mathematical hybrid function syntax, but in Python, we're limited
to explicitly using an if. If I were coding this, and I'm not, I'd wrap
it in a function. One advantage of a state variable rather than a
continuous time function is that we can do this:

def accel(state):
return {NO_BRAKING: 0.0,
LOW_BRAKING: 0.2,
MID_BRAKING: 0.425,
HIGH_BRAKING: 0.85}[state]

That could be done, but with .get() to allow a default. I may well do that.

Given that problem, we actually only care about LOW_BRAKING and
MID_BRAKING. The problem becomes quite simple:

Right. The other state is "zero distance and zero loss of speed".

At t=0, the train is travelling at u m/s and the brakes are applied with
acceleration of 0.2m/s^2 for one second, then 0.425m/s^2 for an
additional one second. What is the speed of the train after those two
seconds, and the distance travelled.

This becomes a simple question for the four standard equations of motion:

(1) v = u + at
(2) s = 1/2(u + v)t
(3) s = ut + 1/2(at^2)
(4) v^2 = u^2 + 2as

Only (1) and (3) are needed.

Okay, what's u here? Heh.

The distance travelled in the first second is:

s1 = u - 0.1 m

and the speed of the train at that time becomes:

v = u - 0.2 m/s

Applying this for the second second gives:

s2 = (u - 0.2) - 0.2125 m

v = (u - 0.2) - 0.425 m/s

and the total distance:

s = s1 + s2


No programming required, just a calculator

Given that solving the entire problem is barely five lines, writing code
to do so is probably unnecessary.

This is one small part of a larger piece of code (which, in this
instance, is trying to answer this question: "If I apply the brakes
now and keep them on until the next curve, will I be way below the
curve's speed limit?" - if yes, don't apply the brakes now).

The only thing I haven't done is check that the train isn't travelling so
slowly that it comes to a complete halt within two seconds. I leave that
as an exercise. (Hint: if the final speed is negative, the train came to
a halt.)

Yeah. Or just let that go into the next part of the calculation, which
works out quadratically how much distance -> how much time -> how much
remaining speed, which may have no solution ie it doesn't reach that
point. That part's fine.

Whatever you used to post this, ate the indentation.

I posted that part without indentation, yeah. Gmail eats tabs.
Normally I manually replace them with spaces for posting, but since
this whole section was at the same indentation level, I didn't bother.
It is one of the problems with Gmail, though, and if I weren't running
this account on multiple systems of different OSes, I'd probably use a
dedicated client.

Using sequence unpacking just to save a newline is naughty

dist, speed = (curspeed - 0.1) + (curspeed - 0.4125), curspeed - 0.625

is an abuse of syntax, not far removed from:

dist = (curspeed - 0.1) + (curspeed - 0.4125); speed = curspeed - 0.625

It will be much more comprehensible written as two statements.

Fair enough. That's why I asked. Is it naughty enough to break into
two statements, or is it better to combine it into a single multiple
assignment?

It could have, but doesn't. Is your aim to build a general purpose
Newtonian linear motion solver, or to solve this one problem?

To solve this one problem, as part of a simulator. For any purpose
beyond what you can see in the code itself, you'd have to ask my
brother; I don't have the full details on that.

Thanks for the advice! Flamage welcome, I'm wearing the asbestos
that's just been removed from the theatre here...

ChrisA

 

[Rustom Mody]

Haskell has nifty pattern-matching syntax for this that looks quite close
to the mathematical hybrid function syntax, but in Python, we're limited
to explicitly using an if. If I were coding this, and I'm not, I'd wrap
it in a function. One advantage of a state variable rather than a
continuous time function is that we can do this:
def accel(state):
return {NO_BRAKING: 0.0,
LOW_BRAKING: 0.2,
MID_BRAKING: 0.425,
HIGH_BRAKING: 0.85}[state]

Neat
I would put the dict in a variable. And those _BRAKINGs are GALLing me!

breaking = {NO:0.0, LOW:0.2, MID:0.425:, HIGH:0.85}
def accel(state): return breaking[state]

<Irony>
In using Haskell, I often wish for dicts especially python's nifty
dict-literals
</Irony>

 

[Ian Kelly]

It actually is a smooth increase in deceleration, but I'm operating
the simulator on a 1s period, so it's actually an average across the
first second, and an average across the next second...

Then your computation is incorrect and will systematically
underestimate the stopping distance. Assuming for simplicity that the
acceleration actually increases linearly until it reaches maximum,
picture the velocity graph between, say, t=0 and t=1s. You are
modeling it as a straight line segment. However, it would actually be
part of a quadratic curve connecting the same points, convex upwards.
The line segment is short-cutting the curve between the two points.
The distance traveled is the integral of the curve, and it is easy to
see that the integral of the line segment is less than the integral of
the actual curve.

Okay, what's u here? Heh.

u is the initial velocity; v is the velocity after accelerating at a for time t.

 

[David Hutto]

u is the initial velocity; v is the velocity after accelerating at a for
time t.

This assumes that the viscosity is in a state of superfluidity, and in a
perfect state between itself, and it's traveling environment.

 

[Ian Kelly]

Haskell has nifty pattern-matching syntax for this that looks quite close
to the mathematical hybrid function syntax, but in Python, we're limited
to explicitly using an if. If I were coding this, and I'm not, I'd wrap
it in a function. One advantage of a state variable rather than a
continuous time function is that we can do this:
def accel(state):
return {NO_BRAKING: 0.0,
LOW_BRAKING: 0.2,
MID_BRAKING: 0.425,
HIGH_BRAKING: 0.85}[state]

Neat
I would put the dict in a variable. And those _BRAKINGs are GALLing me!

breaking = {NO:0.0, LOW:0.2, MID:0.425:, HIGH:0.85}
def accel(state): return breaking[state]

If I were doing this in Python 3.4 I would be sorely tempted to use an
Enum for the state. Then the acceleration could just be an attribute
(or method) of the state itself!

class BrakingState(Enum):
no_braking = 0.0
low_braking = 0.2
mid_braking = 0.425
high_braking = 0.85

@property
def acceleration(self):
return self.value
0.2


Alternatively the __init__ method could set the acceleration directly
from the value(s) passed in, as in the Planet example:

https://docs.python.org/3/library/enum.html#planet

The problem I have with either of these approaches though is that if
two distinct states happen to have the same acceleration, they will be
considered aliases for the same state. This is because the
acceleration value is doing double duty as a parameter of the state
and also as its identity. Likewise in the Planet example, if a new
planet happened to have the same mass and radius as Jupiter, it would
be considered the same planet as Jupiter. I haven't managed to work
out a wholly satisfying way to avoid this.

 

[Ian Kelly]

This assumes that the viscosity is in a state of superfluidity, and in a
perfect state between itself, and it's traveling environment.

I fail to see how this is relevant. I would assume that the amount of
friction is already modeled in the acceleration constants; if it were
zero then the brakes would be nonfunctional and the train would not be
able to accelerate or decelerate at all. In any case, a change in
friction simply works out to a change in acceleration. The equations
above still hold true.

 

[Chris Angelico]

Then your computation is incorrect and will systematically
underestimate the stopping distance. Assuming for simplicity that the
acceleration actually increases linearly until it reaches maximum,
picture the velocity graph between, say, t=0 and t=1s. You are
modeling it as a straight line segment. However, it would actually be
part of a quadratic curve connecting the same points, convex upwards.
The line segment is short-cutting the curve between the two points.
The distance traveled is the integral of the curve, and it is easy to
see that the integral of the line segment is less than the integral of
the actual curve.

..... great.

Okay. I never studied calculus, so this is beyond my expertise. Is
this going to make a majorly significant difference to the end result?
If so, I guess the code's going to have to be a whole lot more
sophisticated, which means I need to learn a whole lot more maths in
order to write it. And I'm still trying to find time to get familiar
with systemd (having jumped on the Upstart bandwagon and now find
myself backing a losing horse, if you'll forgive a mixed metaphor) and
Cython (just need an excuse for that one).

ChrisA

 

[Steven D'Aprano]

On Tue, 01 Apr 2014 16:01:40 +1100, Chris Angelico wrote:

[...]

It actually is a smooth increase in deceleration, but I'm operating the
simulator on a 1s period, so it's actually an average across the first
second, and an average across the next second...

Hmmm. A 1-second resolution doesn't really sound too great to me.

Suppose the deceleration increases linearly from 0 to 0.85 m/s over two
seconds. Averaging it in the way you suggested, we get a total distance
of:

2*u - 0.5125

(see my previous post for details) where u is measured in metres per
second. Multiply by seconds to get the units right. For a Japanese bullet
train where u = 320 km/hr that corresponds to

py> 2*88.888889 - 0.5125
177.26527800000002

metres.

Now let's do it properly! Given our assumption that the deceleration is
linear, the jerk will be constant:

j = Δa/Δt
= (0.85 - 0)/2
= 0.425 m/s^3

Let's start integrating!

py> from sympy import integrate
py> from sympy.abc import t
py> j = '0.425'
py> a = integrate(j, t)
py> v = 88.888889 - integrate(a, t)
py> s = integrate(v, (t, 0, 2))
py> s
177.211111333333

compared to 177.26527800000002 calculated the rough way. That's not bad,
only about 5cm off! Effectively, your rough calculation was accurate to
one decimal place.

Of course, if the equation for acceleration was more complex, the
approximation may not be anywhere near as good.


[...]

Okay, what's u here? Heh.

They're *standard* equations of motion. Weren't you paying attention
through the, oh, three years of high school where they teach this?

s = displacement (distance)
t = time
u = initial velocity
v = final velocity
a = acceleration

[...]

Fair enough. That's why I asked. Is it naughty enough to break into
two statements, or is it better to combine it into a single multiple
assignment?

Since they are unrelated assignments, of two different variables, they
should be written as two separate assignments, not one using sequence
unpacking.

 

[Chris Angelico]

The difference in our opinions, seems to be that there is an initial resting
state, and not at an already accelerated motion that has reached it's
maximum capacity.


So there is a dynamic in my mind's eye, where the object is at a "resting"
point initially, and either the environment, or the object can maneuver
their own viscosity in relation to the other.

The initial state, in this problem, is of a vehicle moving at a known
speed (an input to the formula; it's a variable, but one that we know
at that point). Friction, viscosity, etc, etc, etc are all handled
elsewhere; these trains are generally way overpowered, and the onboard
computer caps the power and non-emergency braking such that the change
in speed never exceeds 0.85 m/s/s. I don't know how exactly it goes
about that, but for the purposes of this test, we can assume that it's
able to achieve that acceleration.

ChrisA