facebookresearch / facebookresearch/fairseq2

`dynamic_bucket` Python signature seems to be wrong

Open
#1,103 0 comments 1 reaction 1 assignee Claimed by @artemru View on GitHub
bug data pipeline
Dominant language
Python
Stars
1.1k
Forks
144
Avg merge
4d 1h
Merged PRs (30d)
1

Description

**Describe the bug:**

When calling `dynamic_bucket` on a data pipeline, I am getting an "incompatible function arguments" error.
```
TypeError: dynamic_bucket(): incompatible function arguments. The following argument types are supported:
1. (self: fairseq2n.bindings.data.data_pipeline.DataPipelineBuilder, threshold: float, fn: Callable[[Any], float], bucket_creation_fn: Optional[Callable[[List[Any]], Tuple[List[Any], List[Any]]]] = None, min_num_examples: Optional[int] = None, max_num_examples: Optional[int] = None, drop_remainder: bool = False) -> fairseq2n.bindings.data.data_pipeline.DataPipelineBuilder

Invoked with: ; kwargs: threshold=5.0, cost_fn=
```

**Describe how to reproduce:**

```Python
from typing import Any
from fairseq2.data import read_sequence

def my_cost_fn(item: Any) -> float:
return float(item)

items = list(range(10))

builder = read_sequence(items)
builder.dynamic_bucket(threshold=5.0, cost_fn=my_cost_fn)
pipeline = builder.and_return()
bucketed = list(pipeline)
print(bucketed)

assert bucketed == [[0, 1, 2, 3], [4, 5], [6], [7], [8], [9]]
```

**Describe the expected behavior:**
I expected no error to happen and assertions to pass.

**Environment:**
I am using `fairseq2==0.4.4`, but the error seems to persist in the main branch.

**Additional Context:**
What is confusing is that I am using exactly the signature described in the Python interface (https://github.com/facebookresearch/fairseq2/blob/19aeff12c16229dfa55e9c4688a585e0ece9e548/src/fairseq2/data/_data_pipeline.py#L238) and in the documentation (https://facebookresearch.github.io/fairseq2/nightly/basics/data_pipeline.html#bucketing). But the underlying C++ code (https://github.com/facebookresearch/fairseq2/blob/19aeff12c16229dfa55e9c4688a585e0ece9e548/native/src/fairseq2n/data/dynamic_bucket_data_source.h#L24) expects the argument to be called just `fn`.

Please reconcile these interfaces. Having to consult the native code for writing simple stuff is annoying.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.