[PR #1597] [MERGED] fix(mlsysim): correct unit conversion in calc_monthly_egress_cost #9209

New Issue

GiteaMirror · 2026-05-03T01:28:28-05:00

GiteaMirror commented

2026-05-03 01:28:28 -05:00

📋 Pull Request Information

Original PR: https://github.com/harvard-edge/cs249r_book/pull/1597
Author: @Shashank-Tripathi-07
Created: 4/28/2026
Status: ✅ Merged
Merged: 4/29/2026
Merged by: @profvjreddi

Base: dev ← Head: fix/egress-cost-unit-bug

📝 Commits (1)

4e1301f fix(mlsysim): correct unit conversion in calc_monthly_egress_cost

📊 Changes

2 files changed (+86 additions, -1 deletions)

View changed files

📝 mlsysim/mlsysim/core/formulas.py (+2 -1)
📝 mlsysim/tests/test_formulas.py (+84 -0)

📄 Description

Summary

calc_monthly_egress_cost multiplied monthly_bytes (in bytes) by the raw cost_per_gb number without any unit conversion
This produces a result ~1,000,000,000x too large (e.g., $1.87 trillion instead of $233 for 1 MB/s at $0.09/GB)
Fix: use _ensure_unit to attach dollar/gigabyte to cost_per_gb, then convert to dollar/byte before multiplying

Root cause

# before -- wrong: monthly_bytes is in bytes, cost_per_gb is raw $/GB number
monthly_bytes = b_s * (30 * ureg.day)   # e.g. 2,592,000,000,000 byte
cost = monthly_bytes * cost_per_gb       # byte * 0.09 (raw) -- no unit conversion!

# after -- correct: cost_per_gb gets units, then converted to $/byte
cost_rate = _ensure_unit(cost_per_gb, ureg.dollar / ureg.gigabyte, "cost_per_gb")
cost = monthly_bytes * cost_rate.to(ureg.dollar / ureg.byte)

Tests added

TestMonthlyEgressCost -- 4 tests: known-answer (raw + Quantity), zero bandwidth, linear scaling
TestFleetTCO -- 3 tests: known-answer, zero quantity, linear scaling
TestMTBFNode -- 3 tests: single component, two GPUs halves MTBF, mixed components

All three functions previously had zero test coverage in test_formulas.py.

Test plan

pytest mlsysim/tests/test_formulas.py -v passes (54 tests, was 43)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/harvard-edge/cs249r_book/pull/1597 **Author:** [@Shashank-Tripathi-07](https://github.com/Shashank-Tripathi-07) **Created:** 4/28/2026 **Status:** ✅ Merged **Merged:** 4/29/2026 **Merged by:** [@profvjreddi](https://github.com/profvjreddi) **Base:** `dev` ← **Head:** `fix/egress-cost-unit-bug` --- ### 📝 Commits (1) - [`4e1301f`](https://github.com/harvard-edge/cs249r_book/commit/4e1301f9d6634f5a5a73ff13e41a8efa09c0ef08) fix(mlsysim): correct unit conversion in calc_monthly_egress_cost ### 📊 Changes **2 files changed** (+86 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `mlsysim/mlsysim/core/formulas.py` (+2 -1) 📝 `mlsysim/tests/test_formulas.py` (+84 -0) </details> ### 📄 Description ## Summary - `calc_monthly_egress_cost` multiplied `monthly_bytes` (in bytes) by the raw `cost_per_gb` number without any unit conversion - This produces a result ~1,000,000,000x too large (e.g., \$1.87 **trillion** instead of \$233 for 1 MB/s at \$0.09/GB) - Fix: use `_ensure_unit` to attach `dollar/gigabyte` to `cost_per_gb`, then convert to `dollar/byte` before multiplying ## Root cause ```python # before -- wrong: monthly_bytes is in bytes, cost_per_gb is raw $/GB number monthly_bytes = b_s * (30 * ureg.day) # e.g. 2,592,000,000,000 byte cost = monthly_bytes * cost_per_gb # byte * 0.09 (raw) -- no unit conversion! # after -- correct: cost_per_gb gets units, then converted to $/byte cost_rate = _ensure_unit(cost_per_gb, ureg.dollar / ureg.gigabyte, "cost_per_gb") cost = monthly_bytes * cost_rate.to(ureg.dollar / ureg.byte) ``` ## Tests added - `TestMonthlyEgressCost` -- 4 tests: known-answer (raw + Quantity), zero bandwidth, linear scaling - `TestFleetTCO` -- 3 tests: known-answer, zero quantity, linear scaling - `TestMTBFNode` -- 3 tests: single component, two GPUs halves MTBF, mixed components All three functions previously had zero test coverage in `test_formulas.py`. ## Test plan - [ ] `pytest mlsysim/tests/test_formulas.py -v` passes (54 tests, was 43) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

GiteaMirror added the pull-request label 2026-05-03 01:28:28 -05:00

GiteaMirror closed this issue

2026-05-03 01:28:30 -05:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/cs249r_book#9209