Skip to content

Commit 356e1d4

Browse files
committed
Fix live-preset dedup/kwargs and tunable memory cap in phi3 layout-map test
- test_layout_map_live_presets dedup key now includes rope_scaling_type, so phi3_mini_128k_instruct_en (SuScaled RoPE) is no longer collapsed into the same width-class as phi3_mini_4k_instruct_en (plain RoPE) and discarded before being built. - init_kwargs for the live-preset build now uses the full preset config (minus "dtype") instead of a dims-only allowlist, so rope_scaling_type/ short_factor/long_factor actually reach the model instead of silently defaulting to plain RoPE. - Tier-3 memory-budget cap is now tunable via the KERAS_HUB_DISTRIBUTION_TEST_MEM_BUDGET env var (defaults to the existing 300MB, so local behavior is unchanged) instead of a hardcoded ceiling no real preset can ever fit under. - Memory-estimate formula now counts both the input embedding table and the untied reverse_embeddings weight (Phi3 always uses tie_weights=False), instead of a single vocab*hidden term.
1 parent 2b0c82a commit 356e1d4

1 file changed

Lines changed: 56 additions & 21 deletions

File tree

keras_hub/src/models/phi3/phi3_backbone_test.py

Lines changed: 56 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import gc
22
import json
3+
import os
34
import re
45

56
import keras
@@ -292,18 +293,26 @@ def test_layout_map_live_presets(self):
292293
self.skipTest("`ModelParallel` testing requires the Jax backend.")
293294

294295
# Fetch every preset's config only (no weights), then dedupe by the
295-
# divisibility-relevant dims so width-classes that share a config
296-
# (both registered Phi3 presets are the same "mini" architecture at
297-
# different context lengths) are only built once per mesh shape -- a
298-
# memory/time necessity under CI resource limits, while every preset
299-
# in the registry is still fetched and evaluated, preserving full
300-
# registry coverage.
296+
# divisibility-relevant dims so width-classes that share both a
297+
# dims profile AND an architecture (e.g. exact duplicate registry
298+
# entries) are only built once per mesh shape -- a memory/time
299+
# necessity under CI resource limits, while every preset in the
300+
# registry is still fetched and evaluated, preserving full registry
301+
# coverage. `rope_scaling_type` is included alongside the raw dims
302+
# because the two registered Phi3 presets share identical dims but
303+
# differ in RoPE architecture: `phi3_mini_4k_instruct_en` uses plain
304+
# RoPE (`rope_scaling_type=None`) while `phi3_mini_128k_instruct_en`
305+
# uses SuScaled RoPE (`rope_scaling_type="su"`) to reach its longer
306+
# context length -- without this key, the 128k preset would be
307+
# discarded as a "duplicate" of the 4k preset before ever being
308+
# built, and its distinct architecture would go untested.
301309
dim_keys = (
302310
"vocabulary_size",
303311
"num_query_heads",
304312
"num_key_value_heads",
305313
"hidden_dim",
306314
"intermediate_dim",
315+
"rope_scaling_type",
307316
)
308317
width_classes = {} # dedupe key -> (config dict, [preset names])
309318
fetch_failures = []
@@ -380,19 +389,25 @@ def test_layout_map_live_presets(self):
380389
skip_reasons.append(reason)
381390
continue
382391

383-
# Memory-budget guard: full-scale presets cannot be
384-
# built locally under CI resource limits. Estimate this
385-
# width-class's single-decoder-block bf16 footprint
386-
# (embedding table + attention projections + one FFN
387-
# block's 3 matrices, times a 3x safety margin for
388-
# JAX/XLA transient copies during construction/
392+
# Memory-budget guard: full-scale presets cannot always
393+
# be built under constrained local/CI resource limits.
394+
# Estimate this width-class's single-decoder-block bf16
395+
# footprint (embedding table + attention projections +
396+
# one FFN block's 3 matrices, times a 3x safety margin
397+
# for JAX/XLA transient copies during construction/
389398
# resharding) and skip the actual build if it exceeds a
390-
# conservative local threshold. The config-fetch, dedup,
391-
# and divisibility-skip logic above still exercises
392-
# every registry preset either way; only the expensive
393-
# build+assert step is capped.
399+
# conservative threshold. The config-fetch, dedup, and
400+
# divisibility-skip logic above still exercises every
401+
# registry preset either way; only the expensive
402+
# build+assert step is capped. Phi3's `token_embedding`
403+
# always uses `tie_weights=False` (see
404+
# `Phi3Backbone.__init__`), so a separate
405+
# `reverse_embeddings` weight of the same
406+
# `vocabulary_size * hidden_dim` size always exists in
407+
# addition to the input embedding table -- both terms
408+
# are counted below, not just one.
394409
est_params = (
395-
cfg["vocabulary_size"] * cfg["hidden_dim"]
410+
2 * cfg["vocabulary_size"] * cfg["hidden_dim"]
396411
+ 3 * cfg["hidden_dim"] * cfg["intermediate_dim"]
397412
# Attention q/k/v/o projections: upper-bounded as
398413
# four hidden_dim x hidden_dim matrices (exact for
@@ -401,7 +416,18 @@ def test_layout_map_live_presets(self):
401416
+ 4 * cfg["hidden_dim"] * cfg["hidden_dim"]
402417
)
403418
est_bytes = est_params * 2 * 3 # bf16 * safety margin
404-
max_local_bytes = 300 * 1024 * 1024 # 300MB
419+
# Tunable via env var so CI or a larger machine can opt
420+
# into exercising width-classes that a 300MB default
421+
# would always skip (the embedding table alone exceeds
422+
# 300MB for any real Phi3 preset, so the default keeps
423+
# today's local-run behavior unchanged while allowing
424+
# real full-scale verification elsewhere).
425+
max_local_bytes = int(
426+
os.environ.get(
427+
"KERAS_HUB_DISTRIBUTION_TEST_MEM_BUDGET",
428+
300 * 1024 * 1024,
429+
)
430+
)
405431
if est_bytes > max_local_bytes:
406432
reason = (
407433
f"{combo_label}: estimated build memory "
@@ -428,9 +454,18 @@ def test_layout_map_live_presets(self):
428454
distribution = keras.distribution.ModelParallel(
429455
layout_map=layout_map, batch_dim_name="batch"
430456
)
431-
init_kwargs = {
432-
k: v for k, v in cfg.items() if k in dim_keys
433-
}
457+
# `cfg` is a full serialized `get_config()` dict, which
458+
# may include a `"dtype"` key (a serialized dtype-policy
459+
# dict, only present for quantized presets) -- drop it
460+
# so the explicit `dtype="bfloat16"` override below
461+
# doesn't collide with a duplicate keyword argument.
462+
# Using the full config (not a dims-only allowlist)
463+
# preserves real architecture flags such as
464+
# `rope_scaling_type`/`rope_scaling_short_factor`/
465+
# `rope_scaling_long_factor`, which distinguish
466+
# `phi3_mini_128k_instruct_en`'s SuScaled-RoPE
467+
# architecture from the plain-RoPE 4k preset.
468+
init_kwargs = {k: v for k, v in cfg.items() if k != "dtype"}
434469
init_kwargs["num_layers"] = 1
435470
with distribution.scope():
436471
model = Phi3Backbone(dtype="bfloat16", **init_kwargs)

0 commit comments

Comments
 (0)