#20 - Small no-ops: tempo inverted {1} else {1}, measure() fa > 0 makes fault_rate always 0, Relabel only shuffles class labels 0-15 - Breadway/omniastra

Breadway commented

2026-10-06 15:14:34 +08:00

Owner

How it was found

Code review of the omniastra repo (findings supplied by the user, 2026-10-06), tempo.rs:200 for the first item. Not re-verified by me in this session.

Confidence it is a bug

High for the first two (tautological code), medium for the third (may be a deliberate limit).

Explanation

* if inverted {1} else {1} in tempo.rs:200 is a no-op, so the inverted variant is not inverted there.
measure() has fa > 0, which is always true, so fault_rate is always 0.
Relabel only shuffles class labels 0-15.

Suggested fix

Fix the intended values, correct the fault_rate condition, and either extend or document the label range.

## How it was found Code review of the omniastra repo (findings supplied by the user, 2026-10-06), `tempo.rs:200` for the first item. Not re-verified by me in this session. ## Confidence it is a bug High for the first two (tautological code), medium for the third (may be a deliberate limit). ## Explanation - [ ] `* if inverted {1} else {1}` in `tempo.rs:200` is a no-op, so the inverted variant is not inverted there. - [ ] `measure()` has `fa > 0`, which is always true, so `fault_rate` is always 0. - [ ] `Relabel` only shuffles class labels 0-15. ## Suggested fix Fix the intended values, correct the fault_rate condition, and either extend or document the label range.

Breadway commented

2026-10-06 16:02:27 +08:00

Author

Owner

Fixed on local branch fix/review-findings in 0f85302 (not pushed yet). The tempo no-op was at tempo.rs:70 in this tree (not :200): the inverted variant now pairs burn cost and damage inversely (damage + cost = 6) while keeping the rng draws identical across variants, so Normal games are unchanged. measure() now really reports fault_rate and rejects above a new max_fault_rate (default 0, same strictness as before). The class-label range is now an explicit CLASS_LABELS constant with a test. This is my reading of the intended inversion; say if you meant something else.

Fixed on local branch fix/review-findings in 0f85302 (not pushed yet). The tempo no-op was at tempo.rs:70 in this tree (not :200): the inverted variant now pairs burn cost and damage inversely (damage + cost = 6) while keeping the rng draws identical across variants, so Normal games are unchanged. measure() now really reports fault_rate and rejects above a new max_fault_rate (default 0, same strictness as before). The class-label range is now an explicit CLASS_LABELS constant with a test. This is my reading of the intended inversion; say if you meant something else.

Breadway commented

2026-10-06 16:10:57 +08:00

Author

Owner

Decision recorded: keep the inverted burn as damage = 6 - cost. It matches how the creature curve is already inverted (cheap is efficient), and Normal games are unchanged because both random draws still happen in every variant. Included in PR #25.

Breadway closed this issue

2026-10-08 19:55:07 +08:00

Rows
Columns

Small no-ops: tempo inverted {1} else {1}, measure() fa > 0 makes fault_rate always 0, Relabel only shuffles class labels 0-15 #20

How it was found

Confidence it is a bug

Explanation

Suggested fix