facebookresearch / facebookresearch/fairseq2
`dynamic_bucket` Python signature seems to be wrong
- 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
Assessment
This issue has not been assessed yet.