lablup / lablup/mlxcel

test(audio): pin the AudioConfig derived attention geometry

Open Beginner friendly
#1,668 0 comments 0 reactions 0 assignees View on GitHub
priority:low status:ready type:test
Dominant language
Rust
Stars
467
Forks
54
Avg merge
4h 25m
Merged PRs (30d)
310

Description

## Summary

The Gemma 4 `AudioConfig` derives its attention geometry with pure integer arithmetic but has no tests, while the sibling audio config module does. Add direct unit coverage for the derived-geometry methods.

## Background

`AudioConfig` computes `head_dim()`, `max_past_horizon()` (`attention_context_left.saturating_sub(1)`), and `context_size()` from serde defaults. These feed the encoder's attention shaping, and the `saturating_sub` boundary (`attention_context_left == 0`) plus the default `context_size()` are exactly the kinds of edges worth pinning. The file has no `cfg(test)` and no `_tests.rs` sibling; `src/audio/phi4mm/config.rs` shows the inline-test pattern.

## Proposed Solution

Add a small inline `#[cfg(test)]` block (or `_tests.rs`) asserting that defaults produce the documented geometry, that `head_dim` divides as expected, and that the `saturating_sub` boundary holds. No checkpoint and no GPU required.

## Implementation Notes

- `src/audio/config.rs:97-107` defines `head_dim` (`hidden_size / num_attention_heads`), `max_past_horizon` (`attention_context_left.saturating_sub(1)`), and `context_size` (`attention_chunk_size + max_past_horizon() + attention_context_right`).
- Consumers at `src/audio/encoder.rs:618-620` show the intended semantics.
- `src/audio/phi4mm/config.rs` carries inline tests to imitate.

## Acceptance Criteria

- [ ] The three derived-geometry methods have direct unit coverage including the zero boundary

---

## Original Suggestion

### Title: test(audio): pin the AudioConfig derived attention geometry

The Gemma 4 `AudioConfig` derives its attention geometry with pure integer arithmetic but has no tests, while the sibling audio config module does.

## Evidence

- `src/audio/config.rs:94-107` — `head_dim()`, `max_past_horizon()` (`attention_context_left.saturating_sub(1)`), and `context_size()` are pure arithmetic over serde defaults; the file has zero `cfg(test)` and no `_tests.rs` sibling
- `src/audio/phi4mm/config.rs` has inline tests showing the pattern
- Worth pinning: the saturating-sub edge case (`attention_context_left == 0`) and the default `context_size()` (12 + 12 + 0 = 24); consumers at `src/audio/encoder.rs:618-620` show the intended semantics

## Suggested fix

Add a small inline `#[cfg(test)]` block (or `_tests.rs`): defaults produce the documented geometry, `head_dim` division, and the `saturating_sub` boundary. Needs no checkpoint and no GPU.

## Acceptance criteria

- [ ] The three derived-geometry methods have direct unit coverage including the zero boundary

Contributor guide

Open the contributing guide

Research direction

Start with src/audio/config.rs:97-107 and compare the inline-test pattern in src/audio/phi4mm/config.rs. Add direct tests for the default head_dim, max_past_horizon, and context_size values, including the zero attention_context_left boundary. Run the relevant Rust tests; done means all three derived methods have unit coverage without a checkpoint or GPU.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
testing-qa
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
92/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.