Fuyu processing: handle coordinates#113
Conversation
…rocessing-update-coordinates
| def original_to_transformed_w_coords(self, original_coords): | ||
| # apply crop | ||
| cropped_coords = ( | ||
| self._clamp_coords(original_coords, min_value=self.crop_left, max_value=self.crop_right) - self.crop_left | ||
| ) | ||
| # apply scale | ||
| scaled_coords = self._scale_coords(cropped_coords, scale=self.scaled_w / self.original_w) | ||
| # apply pad | ||
| return scaled_coords + self.padding_left | ||
| # Simplified assuming self.crop_left = self.padding_left = 0 | ||
| def original_to_transformed_w_coords(original_coords, scale_w): | ||
| return np.round(original_coords * scale_w).astype(np.int32) |
There was a problem hiding this comment.
I haven't seen crop_top, crop_left, padding_left and padding_top different to zero in the original code (see https://huggingface.co/adept-hf-collab/adept-mm/blob/736c6b570b2a9c0367a3266746fd1f53cfff0a2b/mm-inference-for-hf/multimodal/data/image_utils.py#L65).
| seq = tokens_to_boxes(seq, size) | ||
| seq = tokens_to_points(seq, size) | ||
| seq = seq[None, :] | ||
| # TODO: what if sequence lengths vary? |
There was a problem hiding this comment.
I haven't tested this yet, but if we use batch inference and there are different number of encoded boxes or points in the sequences, the tensor shapes will be different after these transformations. How can we deal with this case?
There was a problem hiding this comment.
Not sure what's best. I can see two ways: either we have a a way of padding the sequences to the max length then concatenating, or we return a list of variable length tensors.
If I've understood the method correctly, this is postprocessing the model outputs and not producing the raw outputs which would be fed back to the model when generating the next token. In this case, do we need to return a batched tensor?
There was a problem hiding this comment.
I think returning a list of variable lengths tensors is fine, there's few things after generation that'd leverage a padded version of it. Just something like
results = []
for seq, size in zip(outputs, target_sizes):
seq = tokens_to_boxes(seq, size)
seq = tokens_to_points(seq, size)
results.append(seq)
There was a problem hiding this comment.
Yes, you are both right, thanks! I've checked in other post-processors that there's no need to keep the same input type if it's not necessary.
|
@amyeroberts @molbap could you please take a first look? There's a TODO about batch handling when coordinates are translated that I'm not sure how to resolve. Other than that, I'll check for potentially unused coordinate handling code that was brought over from the original repo. |
| seq = tokens_to_boxes(seq, size) | ||
| seq = tokens_to_points(seq, size) | ||
| seq = seq[None, :] | ||
| # TODO: what if sequence lengths vary? |
There was a problem hiding this comment.
Not sure what's best. I can see two ways: either we have a a way of padding the sequences to the max length then concatenating, or we return a list of variable length tensors.
If I've understood the method correctly, this is postprocessing the model outputs and not producing the raw outputs which would be fed back to the model when generating the next token. In this case, do we need to return a batched tensor?
…rocessing-update-coordinates
…rocessing-update-coordinates
Co-authored-by: Pablo Montalvo <[email protected]>
Replaces huggingface#27083