Report SKUAware storage reservation in planner stats - #4596
Closed
hammad45 wants to merge 1 commit into
Closed
Conversation
Summary: `_compute_storage` recognises `FixedPercentageStorageReservation`, `HeuristicalStorageReservation` and `InferenceStorageReservation`, and falls through to `reserved_hbm_percent = 0.0` for anything else. `SKUAwareStorageReservation` was not imported into `stats.py` at all, so every successful SKUAware plan reported a reservation of zero. Three things were wrong on each such plan: - `Reserved Memory: HBM: 0.0 GB / 0%` and `Planning Memory: 100%`, regardless of what was actually reserved. - The `Dense Storage (per rank)` and `KJT Storage (per rank)` blocks were suppressed entirely, because the same `isinstance` gate guards their rendering. - Per-rank utilization was computed against `(1 - 0.0) * hbm`, overstating the headroom the planner really had. Observed on a dry run of an APS model: the `HeuristicalStorageReservation(25%)` arm reported `23.751 GB / 25%` while the SKUAware arm of the same model reported `0.0 GB / 0%`. SKUAware reserves absolute bytes anchored to a fixed home SKU, so there is no percentage to read off it. This reports the STATIC base -- `model_base_bytes` when set, otherwise `margin_bytes`, plus `runtime_overhead_bytes` -- expressed as a fraction of per-rank HBM, mirroring the existing `FixedAbsoluteStorageReservation` branch. Dense and kjt are deliberately excluded: they render as their own blocks, and including them here would count them twice. Differential Revision: D116706171
Contributor
|
@hammad45 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D116706171. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
_compute_storagerecognisesFixedPercentageStorageReservation,HeuristicalStorageReservationandInferenceStorageReservation, and fallsthrough to
reserved_hbm_percent = 0.0for anything else.SKUAwareStorageReservationwas not imported intostats.pyat all, so everysuccessful SKUAware plan reported a reservation of zero.
Three things were wrong on each such plan:
Reserved Memory: HBM: 0.0 GB / 0%andPlanning Memory: 100%, regardless ofwhat was actually reserved.
Dense Storage (per rank)andKJT Storage (per rank)blocks weresuppressed entirely, because the same
isinstancegate guards their rendering.(1 - 0.0) * hbm, overstating theheadroom the planner really had.
Observed on a dry run of an APS model: the
HeuristicalStorageReservation(25%)arm reported
23.751 GB / 25%while the SKUAware arm of the same model reported0.0 GB / 0%.SKUAware reserves absolute bytes anchored to a fixed home SKU, so there is no
percentage to read off it. This reports the STATIC base --
model_base_byteswhen set, otherwisemargin_bytes, plusruntime_overhead_bytes-- expressed as a fraction of per-rank HBM, mirroringthe existing
FixedAbsoluteStorageReservationbranch. Dense and kjt aredeliberately excluded: they render as their own blocks, and including them here
would count them twice.
Differential Revision: D116706171