Skip to content

Handle optional tensorboard dependency in timers#16

Open
saforem2 wants to merge 2 commits into
codexfrom
0zv1rc-codex/suggest-improvements-for-repository
Open

Handle optional tensorboard dependency in timers#16
saforem2 wants to merge 2 commits into
codexfrom
0zv1rc-codex/suggest-improvements-for-repository

Conversation

@saforem2

Copy link
Copy Markdown
Owner

Summary

  • expand the python GitHub Actions workflow to install development dependencies and execute the unit and transformer pytest suites
  • add development documentation (quickstart guide and contributing guidelines) alongside broadened runtime requirements
  • introduce a lightweight DeepSpeed compatibility shim and adjust tests to skip GPU-dependent cases when the accelerator stack is unavailable
  • ensure timers gracefully handle missing tensorboard installations so CPU-only environments can execute the test suite

Testing

  • pytest tests/unit_tests
  • pytest tests/transformer

https://chatgpt.com/codex/tasks/task_e_68e857249054832abaafff0491ff7eab

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Sorry @saforem2, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting

Comment on lines +1 to +3
from .transformer_layer import ParallelTransformerLayer

__all__ = ["ParallelTransformerLayer"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P0 Badge Fix broken re-export of ParallelTransformerLayer

The new alias module imports ParallelTransformerLayer from transformer_layer, but that file only defines TransformerLayer. Importing megatron.core.transformer.parallel_transformer_layer therefore raises ImportError before any tests run. The same mistake appears in the other parallel_* wrappers (parallel_transformer_block, parallel_attention, parallel_mlp), so none of these modules can be imported. The aliases need to import the real classes (e.g. TransformerLayer as ParallelTransformerLayer) or reference the correct module under megatron.model.transformer.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant