Skip to content

[CICD] Expand Metax core unit coverage for Megatron#58

Merged
Darryl233 merged 228 commits into
flagos-ai:mainfrom
BrianPei:metax-test
Jul 2, 2026
Merged

[CICD] Expand Metax core unit coverage for Megatron#58
Darryl233 merged 228 commits into
flagos-ai:mainfrom
BrianPei:metax-test

Conversation

@AlexMa616

Copy link
Copy Markdown
Collaborator

Description

Expand Metax core unit test coverage for Megatron by adding broad, targeted coverage across core training, checkpointing, inference, tensor-parallel, pipeline-parallel, optimizer, FSDP, and utility paths. The added tests focus on CPU/Metax-compatible execution paths and mock CUDA-only behavior where needed, improving core coverage without changing source logic.

The changes also stabilize the current Metax CI unit test workflow by isolating global test state, avoiding CUDA-only assumptions in coverage tests, and keeping the unit test matrix aligned with the expected shared CI structure.

Type of change

  • New feature (non-breaking change which adds functionality)
  • Infra/Build change (changes to CI/CD workflows or build scripts)
  • Bug fix
  • Code refactoring
  • Documentation change
  • Breaking change

Changes

Metax core coverage expansion: Added high-yield unit coverage for Megatron core paths, including dynamic inference, training helpers, checkpointing, tensor parallelism, pipeline communication, distributed optimizer, FSDP buffers, and core utility surfaces.

Metax-compatible test stabilization: Adjusted tests to avoid CUDA-only assumptions where possible, isolate global test state, and make mocked distributed/CUDA behavior deterministic in the current Metax CI image.

Dynamic inference coverage: Added coverage for dynamic text generation server endpoints, request lifecycle paths, dynamic context/cache handling, graph setup/reset paths, and KV block allocator behavior.

Training and checkpointing coverage: Added coverage for training state helpers, gradient finalization/scheduler paths, checkpoint metadata/version handling, distributed checkpointing utilities, timers, and exchange helpers.

Parallelism and optimizer coverage: Added coverage for tensor-parallel RNG/mapping helpers, pipeline communicator/schedule paths, distributed optimizer static helpers, FSDP buffer/grouping paths, and related CPU-safe surfaces.

Checklist

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have kept CI workflow changes aligned with existing shared workflow patterns
  • My changes generate no new expected warnings beyond existing CI/test warnings
  • I have added/updated tests or test guards needed for Metax CI compatibility
  • Metax core unit tests pass in CI
  • Unit test matrix has been restored for the expected groups
  • I have made corresponding changes to the documentation

AlexMa616 added 30 commits May 22, 2026 12:45
):
for param in full_params_list:
if param in bucket.params:
if any(param is bucket_param for bucket_param in bucket.params):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not modify the core source code only to make tests pass

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restoration complete.

Returns:
The expert layer offset for the current EP rank.
"""
if num_experts is None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function signature/docstring allows None; without this guard, the expert state dict would be incorrectly filtered out.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexMa616 What do you mean incorrectly filtered out? If you don't do the modification, when num_experts is None the function still returns 0. This modification doesn't change the behavior?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right,updated.

Returns:
The processed state dictionary with rewritten expert keys.
"""
if num_experts is None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function signature/docstring allows None; without this guard, the expert state dict would be incorrectly filtered out.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same problem

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

return torch.cuda.temperature()
try:
return torch.cuda.temperature()
except ImportError:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a compatibility fallback for when the NVML API is unavailable in the CUDA environment.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexMa616 when NVML API is unavailable?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should expose the exception if the NVML API is really unavailable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right,updated.

@Darryl233 Darryl233 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@lxd-cumt lxd-cumt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -240,6 +240,7 @@ def is_fp16_supported(self):
return True

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if cpu mkldnn fp16 is not supported, return false here

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

@Darryl233 Darryl233 merged commit 5b1fa01 into flagos-ai:main Jul 2, 2026
25 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants