1
\$\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]
pacmaninbw
26.2k13 gold badges47 silver badges113 bronze badges
asked May 31, 2023 at 9:40
\$\endgroup\$
4
  • 4
    \$\begingroup\$ It is your responsibility to know whether your code is working \$\endgroup\$ Commented May 31, 2023 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\$ Commented May 31, 2023 at 16:35
  • \$\begingroup\$ Yeah, I think it works correctly, I just would like to improve the code if possible \$\endgroup\$ Commented May 31, 2023 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\$ Commented Jun 3, 2023 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.

answered May 31, 2023 at 21:41
\$\endgroup\$
0
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

answered Jun 3, 2023 at 14:26
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.