Add Repetition Range ('rep_range') by discordianbelle · Pull Request #888 · dphnAI/sonar

discordianbelle · 2024-12-14T10:39:09Z

Adds a range to the repetition penalties (all samplers under do_penalties)

Counts back from the current token, applying to all output tokens within range, then prompt tokens if the range extends that far

The most expensive operations are just simple slicing operations which are relatively fast in PyTorch.

AlpinDale

Thanks for the PR. There's a few issues in its current state, as outlined in my comments. Mainly, this seems designed for sequential inference, and will fail for batched inference, due to no handling for per-sequence differences within a batch.

Also please run ./formatting.sh to fix linting issues.

AlpinDale · 2024-12-14T22:18:03Z

 _MAX_TEMP = 1e-2

-APHRODITE_NO_DEPRECATION_WARNING = envs.APHRODITE_NO_DEPRECATION_WARNING
+APHRODITE_NO_DEPRECATION_WARNING = bool(int(os.environ.get("APHRODITE_NO_DEPRECATION_WARNING", "0")))


Why this change?

After installing as editible and then modifying the files, I was getting circular import errors. As far as I could tell it was because envs.py is in ./aphrodite/ and not ./aphrodite/common/

I was trying to make the least impactful change that still let it run, so I didn't want to move envs.py

AlpinDale · 2024-12-14T22:18:59Z

        if self.repetition_penalty < 1.0:
            raise ValueError("repetition_penalty must be in [1, inf), got "
                             f"{self.repetition_penalty}.")
+        if self.rep_range is not None and self.rep_range < 1:


We should probably allow 0 for infinite (or all tokens) range. Unless other inference software do it this way.

You're absolutely right, I'll implement on Monday

AlpinDale · 2024-12-14T22:19:14Z

 # If enabled, we switch to a more performant implementation
 # of top-k and top-p
-APHRODITE_USE_SAMPLING_KERNELS = envs.APHRODITE_USE_SAMPLING_KERNELS
+APHRODITE_USE_SAMPLING_KERNELS = bool(int(os.environ.get("APHRODITE_USE_SAMPLING_KERNELS", "0")))


Same as before.

See other instance

AlpinDale · 2024-12-14T22:20:10Z

    logits = torch.where(logits > 0, logits / repetition_penalties,
                         logits * repetition_penalties)

-    # We follow the definition in OpenAI API.


Don't remove comment.

o7
Sorry, danger of AI-assisted programming

AlpinDale · 2024-12-14T22:24:03Z

                    sampling_tensors.frequency_penalties,
-                    sampling_tensors.repetition_penalties)
+                    sampling_tensors.repetition_penalties,
+                    rep_range=rep_range)


Suggested change

rep_range=rep_range)

sampling_tensors.rep_range)

This parameter needs to be added to the sampling_metadata module.

I'll fix on Monday!

AlpinDale · 2024-12-14T22:24:34Z

                     frequency_penalties: torch.Tensor,
-                     repetition_penalties: torch.Tensor) -> torch.Tensor:
+                     repetition_penalties: torch.Tensor,
+                     rep_range: Optional[int] = None) -> torch.Tensor:


Suggested change

rep_range: Optional[int] = None) -> torch.Tensor:

rep_range: torch.Tensor) -> torch.Tensor:

Must be tensorized after it's added to sampling_metadata. If we treat it as a single integer, it'll apply to all sequences within the batch; tensorizing this will allow us to match the batch dimension, in case other sequences may want different ranges.

Great point, will handle

AlpinDale · 2024-12-14T22:28:23Z

+    if rep_range is not None and rep_range > 0:
+        # Just take the last rep_range tokens from output_tokens_tensor
+        # This is much more efficient as we're only looking at recent history
+        output_tokens_tensor = output_tokens_tensor[:, -rep_range:]


This is applying the same range to all sequences in batch, no?

Also creating a new tensor here is probably less efficient. I think the slicing should be done in the bin counting function above this.

Agreed, I'll have to go over it again with batching in mind, and also agreed about rolling in the slicing

AlpinDale · 2024-12-14T22:29:00Z

+        if output_len < rep_range:
+            # Calculate how many prompt tokens we should include
+            prompt_tokens_to_include = min(rep_range - output_len, prompt_end_idx)
+            prompt_tokens_tensor = prompt_tokens_tensor[:, -prompt_tokens_to_include:]


Not all sequences in a batch have the same output len (and consequently may not include the same number of prompt tokens).

Agreed, I'll have to go over it again with batching in mind

AlpinDale · 2024-12-14T22:29:42Z

+            prompt_tokens_tensor = prompt_tokens_tensor[:, -prompt_tokens_to_include:]
+        else:
+            # If we have enough output tokens, ignore prompt completely
+            prompt_tokens_tensor = torch.empty((num_seqs, 0), dtype=torch.long, device=logits.device)


Do we need to do this here? I think most of the range ops can be done in bin counting.

I'll refactor it, thanks again

discordianbelle added 6 commits December 14, 2024 00:03

Update sampler.py for repetition range

7181684

Update sampling_params.py for rep_range

2704c89

Update protocol.py for rep_range

eb05f16

Avoid circular import for simple warning

83bdc2d

fix typo

ffc5b0b

Update sampler.py to avoid circular imports

067d748

AlpinDale requested changes Dec 14, 2024

View reviewed changes

	rep_range: Optional[int] = None) -> torch.Tensor:
	rep_range: torch.Tensor) -> torch.Tensor:

Uh oh!

Uh oh!

Conversation

discordianbelle commented Dec 14, 2024

Uh oh!

AlpinDale left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlpinDale Dec 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AlpinDale left a comment •

edited

Loading

AlpinDale Dec 14, 2024 •

edited

Loading