cactus-compute / cactus-compute/needle

Inference path imports datasets via re-export from dataset.dataset

Open
#40 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
11k
Forks
710
Avg merge
1d 21h
Merged PRs (30d)
25

Description

`import needle` pulls in HuggingFace `datasets` (and its `pyarrow`/`pandas`/`fsspec` tree) even though nothing on the inference path uses it. The tokenizer symbols needed by `run.py` and `__init__.py` are defined in `needle/dataset/tokenizer.py`, but are imported through `needle/dataset/dataset.py`, which imports `datasets` at module level.

### Reproduction

Commit `ffb1c51`:

```bash
python -c "import sys; import needle; print('datasets' in sys.modules)"
# True
```

### Detail

Three sites reach `datasets` on the inference path:

- `needle/__init__.py:14`: `from needle.dataset.dataset import get_tokenizer`
- `needle/model/run.py:11`: `from ..dataset.dataset import get_tokenizer, to_snake_case, DEFAULT_MAX_ENC_LEN, DEFAULT_MAX_GEN_LEN`
- `needle/dataset/__init__.py:2`: `from .dataset import *`

`needle/dataset/dataset.py:15` then imports `datasets` unconditionally at module level.

All four symbols are defined in `needle/dataset/tokenizer.py` and merely re-exported by `dataset.py:19`. `tokenizer.py` itself only needs `sentencepiece` and `tqdm` at module level; `architecture.py` and `constrained.py` have no relative imports at all. So these three lines are the only coupling between inference and the training stack.

The third site matters most: `needle/dataset/__init__.py` executes on *any* `needle.dataset.*` import, so fixing only the first two has no effect.

### Why it matters

`generate()` / `load_checkpoint()` are usable as an inference dependency downstream, but importing them currently requires the training stack to be installed and imported at startup. For a 26M model aimed at constrained environments, that's a real cost for code the inference path never touches.

I appreciate this may be intentional if the Python package is considered a training/research harness with Cactus as the real inference runtime, happy to close if so. But since the fix is behavior-preserving and changes no dependencies or public API, it seemed worth raising.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the import sites in needle/__init__.py, needle/model/run.py, and needle/dataset/__init__.py, then inspect needle/dataset/dataset.py and needle/dataset/tokenizer.py to trace the re-exports. Run the provided Python reproduction before and after the change; done means importing needle no longer places datasets in sys.modules while the listed tokenizer symbols remain available.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
machine-learning
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.