anthropics / anthropics/original_performance_takehome

Two potential bugs in build_mem_image in problem.py

Open
#10 2 comments 13 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
4.2k
Forks
950
PR merge metrics
No merged PRs in 30d

Description

Found while working through the take-home challenge. Two issues in build_mem_image:

1. Line 498: Likely typo in pointer calculation

Current:
inp_values_p = inp_indices_p + len(inp.values) # should be len(inp.indices)

Should be:
inp_values_p = inp_indices_p + len(inp.indices)

Currently works by accident since len(inp.indices) == len(inp.values) == batch_size, but the intent appears to be indices + len(indices) = values_start.

2. Line 512: Slice assignment truncates allocated extra room

Current:
mem[inp_values_p:] = inp.values # truncates list

The function allocates extra room on lines 492-494:
extra_room = len(t.values) + len(inp.indices) * 2 + VLEN * 2 + 32
mem = [0] * (header + len(t.values) + len(inp.indices) + len(inp.values) + extra_room)

But mem[inp_values_p:] = inp.values replaces everything from inp_values_p to end, discarding the extra room. Python slice assignment shrinks the list when the replacement is shorter than the slice.

Should be:
mem[inp_values_p:inp_values_p + len(inp.values)] = inp.values

Additionally, mem[7] = extra_room on line 508 gets immediately overwritten by mem[header:inp_indices_p] = t.values on line 510 (since header = 7), so the extra_room pointer is never actually accessible from the header.

Not blocking for the challenge (computed extra_room_p at runtime as inp_values_p + batch_size), but figured worth flagging.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.