Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Features/streaming penalty #234

Merged
merged 22 commits into from
Jun 5, 2024
Merged

Features/streaming penalty #234

merged 22 commits into from
Jun 5, 2024

Conversation

p-ferreira
Copy link
Contributor

@p-ferreira p-ferreira commented May 17, 2024

  • refactors synapse stream result
  • add streaming data to logs and forward flow
  • refactors protocol to return string chunks in streaming rather than string arrays
  • adapt streaming processor to calculate tokens by chunk
  • drops legacy prompting synapse from repo
  • adds streaming reward function
  • adds a global penalty definition to tasks
  • adds unit tests

@p-ferreira p-ferreira marked this pull request as ready for review May 28, 2024 21:06
@p-ferreira p-ferreira changed the title Features/stream adjustments Features/streaming penalty May 28, 2024
@p-ferreira p-ferreira self-assigned this May 28, 2024
prompting/tasks/task.py Show resolved Hide resolved
prompting/rewards/reward.py Show resolved Hide resolved
@p-ferreira p-ferreira merged commit 834d70c into staging Jun 5, 2024
2 checks passed
Copy link
Collaborator

@dbobrenko dbobrenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +27 to +36
start_time = time.time()

# Calculate the accumulated penalty for the current chunk
accumulated_penalty = sum(
penalty_per_exceeding_chunk if tokens_per_chunk > self.max_tokens_per_chunk else 0
for tokens_per_chunk in response_tokens_per_chunks
)

# Record the timing for this computation
timings.append(time.time() - start_time)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor suggestion is to measure with perf_counter which is more precise and not affected by system clock

Suggested change
start_time = time.time()
# Calculate the accumulated penalty for the current chunk
accumulated_penalty = sum(
penalty_per_exceeding_chunk if tokens_per_chunk > self.max_tokens_per_chunk else 0
for tokens_per_chunk in response_tokens_per_chunks
)
# Record the timing for this computation
timings.append(time.time() - start_time)
start_time = time.perf_counter()
# Calculate the accumulated penalty for the current chunk
accumulated_penalty = sum(
penalty_per_exceeding_chunk if tokens_per_chunk > self.max_tokens_per_chunk else 0
for tokens_per_chunk in response_tokens_per_chunks
)
# Record the timing for this computation
timings.append(time.perf_counter() - start_time)

@p-ferreira p-ferreira mentioned this pull request Jun 5, 2024
@Hollyqui Hollyqui deleted the features/stream-adjustments branch August 2, 2024 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants