0
\$\begingroup\$

someone can help me with this code? I'm trying to compute the average speed every 100 m, starting from a list of spans that contains [distance, time, avg speed].

This is what I've tried to do, and seems to work pretty well (can you confirm?). The problem is that the code is a little bit bulky, and I would like to streamline it if possible.

# list as example
list=[[230,20,11.5],[80,4,20],[300,15,20],[20,1,20],[400,80,5],]


current_dist=0
span_speeds=[]
remaining_time=list[0][1]
current_dist=list[0][0]
speed=list[0][2]
for element in list[1:]:
    while current_dist >= 100:
        span_speeds.append(speed)
        current_dist=current_dist-100
        remaining_time=remaining_time-100/speed
    while current_dist < 100:
        length,time,speed=element
        new_dist=current_dist+length
        if new_dist<100:
            remaining_time=remaining_time+time
            current_dist=new_dist
            break
        tot_time=remaining_time+(100-current_dist)/speed
        span_speeds.append(100/tot_time)        
        remaining_time=time-(100-current_dist)/speed    
        current_dist=new_dist-100    
        continue

# computation about the last span
while current_dist >= 100:
        span_speeds.append(speed)
        current_dist=current_dist-100
        remaining_time=remaining_time-100/speed

This are the results that i'm seeing:

span_speeds = [11.5, 11.5, 16.370106761565832, 20.0, 20, 20, 20.0, 6.451612903225806, 5, 5, 5]
\$\endgroup\$
4
  • 4
    \$\begingroup\$ It is your responsibility to know whether your code is working \$\endgroup\$
    – Reinderien
    May 31 at 12:36
  • \$\begingroup\$ Welcome to Code Review! From the help center page What topics can I ask about here?: "To the best of my knowledge, does the code work as intended?" Can you answer "yes" to that question? \$\endgroup\$ May 31 at 16:35
  • \$\begingroup\$ Yeah, I think it works correctly, I just would like to improve the code if possible \$\endgroup\$
    – Fabio
    May 31 at 18:47
  • \$\begingroup\$ Please do not edit the question, especially the code, after an answer has been posted. Changing the question may cause answer invalidation. Everyone needs to be able to see what the reviewer was referring to. What to do after the question has been answered. You can ask a follow up question with the updated code that has a link back to this question. \$\endgroup\$
    – pacmaninbw
    Jun 3 at 13:39

2 Answers 2

1
\$\begingroup\$

Checking your work: implied total time. Total distance and time in the input example are 1030 and 120. Your code's output speeds can be used to compute total time, along the lines sketched below. Unfortunately, that calculation produces 105 rather than 120, so we know that something is off in your speeds.

speeds = [... your results ...]

last = len(speeds) - 1
tot_time = sum(
    (30 if i == last else 100) / s   # time = dist / speed
    for i, s in enumerate(speeds)
)                                    # 105 seconds

Checking your work: expected partitioning. We can also examine the example input distances and quickly figure out how the partitioning into distance chunks of 100 should play out, as shown in the table below. Notice the last row: it implies that the last four average speeds should be the same -- namely, the speed associated with 400 in the input data. That speed is 5. Unfortunately, your output contains only three values of 5 at the end. I did not bother to figure out where your code went wrong, but my guess is that it occurred when you needed to merge 3 distances to achieve 100 (10 + 20 + 70).

Input distances | Partitions into 100s
--------------------------------------
230             | 100 100 30
80              | 70 10
300             | 90 100 100 10
20              | 20
400             | 70 100 100 100 30

Your speeds (rounded for display here):

11.5, 11.5, 16.4, 20, 20, 20, 20, 6.5, 5, 5, 5

Very quick code review. Your code isn't in functions (it should be). Your code has a cramped, hard-to-read layout (it should add spaces around operators/etc and include blank lines to separate the code into meaningful sections). Your code relies on primitive data objects -- a list of triples -- and thus forces the reader to interpret and remember list-index numbers (it should used data objects that are readable and self-documenting).

How I might start a rewrite. Begin with a meaningful object to represent your data. You have distance-time data representing travel segments/chunks/spans of a moving object (not sure whether there is a proper physics term for this). Vocabulary aside, you might define an object holding distance and time, and then leave speed as a derived attribute. Both for computing the speed and for other parts of your algorithm, it might be useful to include a property indicating whether the span is empty (zero time or distance). For example:

from dataclasses import dataclass

@dataclass
class Span:
    dist: float
    time: float

    @property
    def speed(self):
        return None if self.empty else self.dist / self.time

    @property
    def empty(self):
        return self.time == 0

Next steps. With that data-object defined, you might add some useful behaviors to it, such as the ability for one span to merge with all/part of another (up to some needed amount of distance) or the ability to take a span and a distance-limit and split it apart into two new spans (the second one might be empty).

class Span:
    ...

    def merge_with(self, other, needed = float('inf')):
        # Compute distance we are adding.
        dist = min(other.dist, needed)
        ratio = dist / other.dist

        # Add to self.
        self.dist += dist
        self.time += other.time * ratio

        # Subtract from other.
        ...

    @classmethod
    def split(cls, s, limit):
        remainder = s.dist - limit
        if remainder > 0:
            ratio = remainder / s.dist
            return (
                cls(limit, s.time * (1 - ratio)),
                cls(remainder, s.time * ratio),
            )
        else:
            return (s, cls(0, 0))

With those building blocks in place, writing a function to convert the raw-triples into spans is not necessarily easy, but the resulting code would be a lot more readable.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Thank you, I'll try! \$\endgroup\$
    – Fabio
    Jun 1 at 7:59
1
\$\begingroup\$

Using your suggestions I've created this new code:

from dataclasses import dataclass

@dataclass
class Span:
    dist: float
    time: float

    @property
    def speed(self):
        return None if self.empty else self.dist / self.time

    @property
    def empty(self):
        return self.time == 0
    
    def merge_with(self, other, needed = float('inf')):
        # Compute distance we are adding.
        dist = min(other.dist, needed)
        ratio = dist / other.dist

        # Add to self.
        self.dist += dist
        self.time += other.time * ratio

        # Subtract from other.
        other.dist -= dist
        other.time -= other.time * ratio

    @classmethod
    def split(cls, s, limit):
        remainder = s.dist - limit
        if remainder > 0:
            ratio = remainder / s.dist
            return (
                cls(limit, s.time * (1 - ratio)),
                cls(remainder, s.time * ratio),
            )
        else:
            return (s, cls(0, 0))





def process_spans(spans, limit):
    new_spans = []
    remaining = Span(0,0)  # Distance remaining from the previous merge operation

    for span in spans:
        if remaining.dist > 0:
            needed=limit-remaining.dist
            # Merge the remaining distance with the current span
            remaining.merge_with(span, needed)
            if remaining.dist==limit:
                new_spans.append(remaining)
                remaining=Span(0,0)
            else:
                continue

        while span.dist >= limit:
            # Split the span into smaller spans of the specified limit
            split_spans = Span.split(span, limit)
            new_spans.append(split_spans[0])
            span = split_spans[1]

        else:
            remaining = Span(span.dist,span.time)
    
    new_spans.append(remaining)

    return new_spans



spans = [
    Span(150, 100),
    Span(40, 40),
    Span(140, 70),
    Span(15,15),
    Span(345,30),
    Span(130,13)
]

in_dist_tot = 0
in_time_tot = 0

for in_span in spans:
    in_dist_tot += in_span.dist
    in_time_tot += in_span.time

print(f'Total initial distance: {in_dist_tot}, total initial time: {in_time_tot}\n')


limit = 100

new_spans = process_spans(spans, limit)

dist_tot=0
time_tot=0
print("Span subdivision:")
for span in new_spans:
    print(f"Distance: {span.dist} m, Time: {round(span.time,2)} s")
    dist_tot += span.dist
    time_tot += span.time

print(f'\nTotal distance: {dist_tot}, total time: {round(time_tot,2)}')

This is the result:

Total initial distance: 820, total initial time: 268

Span subdivision:
Distance: 100 m, Time: 66.67 s
Distance: 100 m, Time: 78.33 s
Distance: 100 m, Time: 50.0 s
Distance: 100 m, Time: 34.78 s
Distance: 100 m, Time: 8.7 s
Distance: 100 m, Time: 8.7 s
Distance: 100 m, Time: 8.83 s
Distance: 100 m, Time: 10.0 s
Distance: 20 m, Time: 2.0 s

Total distance: 820, total time: 268.0

It seems to work properly, since the initial time and distance are equivalent. Obviously to calculate the speed it's needed to divide the distance for the time.

A special thanks to @FMc

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.