Skip to content

[refactor] feat: separate data, batch and metric with clear variable names#1339

Closed
tongyx361 wants to merge 7 commits intoverl-project:mainfrom
tongyx361:tyx/refactor/separate-data-batch-metric
Closed

[refactor] feat: separate data, batch and metric with clear variable names#1339
tongyx361 wants to merge 7 commits intoverl-project:mainfrom
tongyx361:tyx/refactor/separate-data-batch-metric

Conversation

@tongyx361
Copy link
Copy Markdown
Collaborator

@tongyx361 tongyx361 commented Apr 30, 2025

What does this PR do?

This PR separates data, batch and metric with clear variable names to avoid reusing the same name data for different variables.

ChangeLog:

  • Rename variables according to loop level and class
    • loop level: (no prefix) -> mini -> micro
    • class:
      • DataProto: data
      • TensorDict: batch
      • dict for metrics: metric_data
  • Simplify code for multi-modal data loading:
non_tensor_select_keys = ["multi_modal_inputs"] if "multi_modal_inputs" in data.non_tensor_batch.keys() else []

Usage:

No change.

Before submitting

  • Did you read the Contribute Guide and finish the code format check?
  • Did you make sure to update the documentations with your changes in the docs especially for breaking config etc?
  • Did you write any test cases if neccessary? Please add CI tests to your new feature.

Additional Info:

  • Issue Number: none
  • Training: FSDP
  • Inference: none

@tongyx361 tongyx361 enabled auto-merge (squash) April 30, 2025 20:21
@tongyx361 tongyx361 mentioned this pull request Apr 30, 2025
3 tasks
@tongyx361 tongyx361 force-pushed the tyx/refactor/separate-data-batch-metric branch from 75eed5c to 730d120 Compare April 30, 2025 23:43
@tongyx361 tongyx361 requested a review from vermouth1992 May 1, 2025 12:53
@tongyx361 tongyx361 closed this May 23, 2025
auto-merge was automatically disabled May 23, 2025 08:48

Pull request was closed

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants