-
Notifications
You must be signed in to change notification settings - Fork 4.4k
varlendataset for thd e2e and benchmark #4832
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
4a9a9dc
add varlendataset for thd e2e and benchmark
xiaoyao0115 2332785
add support for dataset like https://huggingface.co/datasets/allenai/…
xiaoyao0115 a85be94
minor fixes according to the comments
xiaoyao0115 5e9b3e2
some fixes according to the comments
xiaoyao0115 5051382
fix ut
xiaoyao0115 14d94d4
minor fixes
xiaoyao0115 0e48a3b
reanme varlen_bshd_validation to arlen_sbhd_validation
xiaoyao0115 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,6 +89,7 @@ def add_megatron_arguments(parser: argparse.ArgumentParser): | |
| parser = _add_msc_args(parser) | ||
| parser = _add_kitchen_quantization_arguments(parser) | ||
| parser = _add_sft_args(parser) | ||
| parser = _add_varlen_dataset_args(parser) | ||
|
|
||
| parser = _add_fault_injector_args(parser) | ||
|
|
||
|
|
@@ -1554,16 +1555,6 @@ def validate_args(args, defaults={}): | |
| f"to {args.data_parallel_size * args.context_parallel_size}." | ||
| ) | ||
|
|
||
| if args.sequence_packing_scheduler is not None: | ||
| if args.sequence_packing_scheduler == 'dp_balanced': | ||
| total_cp_ranks = args.context_parallel_size | ||
| else: | ||
| total_cp_ranks = args.data_parallel_size * args.context_parallel_size | ||
| assert total_cp_ranks * args.max_seqlen_per_dp_cp_rank >= args.seq_length, ( | ||
| f'Packed sequence buffer size ({total_cp_ranks * args.max_seqlen_per_dp_cp_rank}) ' | ||
| f'must be >= single sequence max length ({args.seq_length})' | ||
| ) | ||
|
|
||
| # disable async_tensor_model_parallel_allreduce when | ||
| # model parallel memory optimization is enabled | ||
| if ( | ||
|
|
@@ -1692,6 +1683,55 @@ def validate_args(args, defaults={}): | |
| args.use_megatron_fsdp | ||
| ), "--ckpt-format fsdp_dtensor is only tested with Megatron FSDP." | ||
|
|
||
| # --use-varlen-dataset: independent of --sft. Cannot be combined with --sft | ||
| # because they are mutually-exclusive top-level dataset selectors that both | ||
| # drive the packed-sequence (THD) path. | ||
| if args.use_varlen_dataset: | ||
| assert not args.sft, ( | ||
| "--use-varlen-dataset and --sft are mutually exclusive; both " | ||
| "select the packed-sequence dataset family. Pick one." | ||
| ) | ||
| if args.varlen_sbhd_validation: | ||
| # ``--dynamic-context-parallel`` ⊥ ``--varlen-sbhd-validation`` is | ||
| # checked in ``GPTDatasetConfig.__post_init__``; only the | ||
| # scheduler check stays here, since ``sequence_packing_scheduler`` | ||
| # is a training-framework flag not stored on the dataset config. | ||
| assert args.sequence_packing_scheduler is None, ( | ||
| "--varlen-sbhd-validation does not use a sequence packing " | ||
| "scheduler; drop --sequence-packing-scheduler." | ||
| ) | ||
| # SBHD validation is a real-data numerical-reference path only; | ||
| # MockVarlenDataset does not implement it. | ||
| assert not args.mock_data, ( | ||
| "--varlen-sbhd-validation is not supported with --mock-data; " | ||
| "SBHD validation requires a real dataset." | ||
| ) | ||
| else: | ||
| # VarlenDataset emits one unpacked sample per __getitem__; it | ||
| # relies on an upstream packing scheduler to group variable-length | ||
| # samples into THD batches. Auto-pick a default scheduler when | ||
| # the user did not request one explicitly: | ||
| # * ``--dynamic-context-parallel`` is already wired to | ||
| # ``default_dynamic_cp`` upstream (see the dynamic-cp block | ||
| # earlier in ``validate_args``). | ||
| # * Otherwise fall back to ``dp_balanced`` (static packing). | ||
| if args.sequence_packing_scheduler is None: | ||
| args.sequence_packing_scheduler = 'dp_balanced' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The normal |
||
|
|
||
| # Packed-sequence buffer-size check. Placed after all scheduler auto-select | ||
| # logic (dynamic-cp and --use-varlen-dataset both set the scheduler above) | ||
| # so it validates the final resolved scheduler; the varlen path picks its | ||
| # default after the earlier generic validation has run. | ||
| if args.sequence_packing_scheduler is not None: | ||
| if args.sequence_packing_scheduler == 'dp_balanced': | ||
| total_cp_ranks = args.context_parallel_size | ||
| else: | ||
| total_cp_ranks = args.data_parallel_size * args.context_parallel_size | ||
| assert total_cp_ranks * args.max_seqlen_per_dp_cp_rank >= args.seq_length, ( | ||
| f'Packed sequence buffer size ({total_cp_ranks * args.max_seqlen_per_dp_cp_rank}) ' | ||
| f'must be >= single sequence max length ({args.seq_length})' | ||
| ) | ||
|
|
||
| # Data blend checks | ||
| assert ( | ||
| args.mock_data | ||
|
|
@@ -4840,13 +4880,64 @@ def _add_sft_args(parser): | |
| '--sft-mock-dataset-config-json', | ||
| type=str, | ||
| default=None, | ||
| help='This config provides the necessary information for the mock dataset. You can either specify a CSV file that contains sequence lengths, where each line stores the length of a sequence, for example: {"mode":"file","path":"/path/to/file"}. Alternatively, you can specify a distribution (currently only supporting lognormal distribution) along with the required parameters, for example, {"mode":"distribution","type":"lognormal","min_seq_len":1024,"max_seq_len":2048,"mean_seq_len":1536,"lognormal_sigma":1.1}, where sigma controls the variability of the lognormal distribution. ' | ||
| help='This config provides the necessary information for the mock dataset. ' | ||
| 'Accepts either an inline JSON literal or a path to a JSON file containing ' | ||
| 'the same schema. You can either specify a CSV file that contains sequence lengths, ' | ||
| 'where each line stores the length of a sequence, for example: ' | ||
| '{"mode":"file","path":"/path/to/file"}. Alternatively, you can specify a distribution ' | ||
| '(currently only supporting lognormal distribution) along with the required parameters, ' | ||
| 'for example, {"mode":"distribution","type":"lognormal","min_seq_len":1024,' | ||
| '"max_seq_len":2048,"mean_seq_len":1536,"lognormal_sigma":1.1}, where sigma controls ' | ||
| 'the variability of the lognormal distribution. ' | ||
| 'If not specified and --mock-data is set, defaults to a lognormal distribution with ' | ||
| 'min_seq_len=seq_length//2, max_seq_len=seq_length, mean_seq_len=seq_length*3//4, lognormal_sigma=1.1.', | ||
| ) | ||
| return parser | ||
|
|
||
|
|
||
| def _add_varlen_dataset_args(parser): | ||
| group = parser.add_argument_group(title='varlen dataset') | ||
| group.add_argument( | ||
| '--use-varlen-dataset', | ||
| action="store_true", | ||
| help='Train with VarlenDataset, a variable-length packed (THD) dataset ' | ||
| 'that consumes instruction-tuning data from a HuggingFace Hub repo id, ' | ||
| 'a local parquet file, or a local jsonl file. Schema (alpaca / sharegpt ' | ||
| '/ openai-messages) is auto-detected from the dataset columns. ' | ||
| 'Mutually exclusive with --sft. Auto-picks a sequence packing ' | ||
| 'scheduler when none is given: ``dp_balanced`` by default, ' | ||
| '``default_dynamic_cp`` when ``--dynamic-context-parallel`` is set. ' | ||
| 'Combine with --mock-data for a synthetic lognormal sequence-length ' | ||
| 'distribution; see --varlen-mock-dataset-config-json.', | ||
| ) | ||
| group.add_argument( | ||
| '--varlen-sbhd-validation', | ||
| action="store_true", | ||
| help='Reference SBHD mode for THD numerical verification. When set, ' | ||
| 'VarlenDataset emits SBHD-style samples right-padded to ' | ||
| '--seq-length (no cu_seqlens, no packing scheduler), so the run can ' | ||
| 'be compared against the THD path to validate correctness. ' | ||
| 'Incompatible with --dynamic-context-parallel and ' | ||
| '--sequence-packing-scheduler.', | ||
| ) | ||
| group.add_argument( | ||
| '--varlen-mock-dataset-config-json', | ||
| type=str, | ||
| default=None, | ||
| help='Mock-dataset config for --use-varlen-dataset --mock-data. ' | ||
| 'Accepts either an inline JSON literal or a path to a JSON file containing ' | ||
| 'the same schema as --sft-mock-dataset-config-json: either ' | ||
| '{"mode":"file","path":"/path/to/lengths.csv"}, ' | ||
| '{"mode":"distribution","type":"lognormal","min_seq_len":1024,' | ||
| '"max_seq_len":2048,"mean_seq_len":1536,"lognormal_sigma":1.1}, or ' | ||
| '{"mode":"verification","data_path":"/prefix/of/IndexedDataset"}. ' | ||
| 'If not specified, defaults to a lognormal distribution with ' | ||
| 'min_seq_len=seq_length//2, max_seq_len=seq_length, ' | ||
| 'mean_seq_len=seq_length*3//4, lognormal_sigma=1.1.', | ||
| ) | ||
| return parser | ||
|
|
||
|
|
||
| def _add_fault_injector_args(parser): | ||
| from megatron.training.config import FaultInjectorConfig | ||
|
|
||
|
|
||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we support either a JSON string or a JSON file path for
sft_mock_dataset_config_jsonandvarlen_mock_dataset_config_json? It is a little bit annoying for users to pass a JSON string via the CLI.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, i'll make the change~