add the forge folder #1387
add the forge folder #1387
Conversation
facebook-github-bot
added
the
CLA Signed
| pipelining_fn: PipeliningFunction | None | ||
| build_optimizers_fn: OptimizersBuilder | ||
| build_lr_schedulers_fn: LRSchedulersBuilder | ||
| build_tokenizer_fn: TokenizerBuilder | None |
There was a problem hiding this comment.
This one we don't need, right?
| float8: Float8 = field(default_factory=Float8) | ||
| mx: MX = field(default_factory=MX) | ||
| comm: Comm = field(default_factory=Comm) | ||
| fault_tolerance: FaultTolerance = field(default_factory=FaultTolerance) | ||
| experimental: Experimental = field(default_factory=Experimental) |
There was a problem hiding this comment.
In the spirit of starting minimal I would leave most of these out for now. Lmk if you disagree though (I am also happy to come back later and add them as needed)
There was a problem hiding this comment.
I can do that. We probably want to keep the Comm for debugging purpose. It is used in init_distributed
| if job_config.experimental.custom_import: | ||
| importlib.import_module(job_config.experimental.custom_import) |
There was a problem hiding this comment.
We probably don't need this, right?
| self.ft_manager = init_ft_manager(job_config) | ||
| # If TorchFT is enabled, the dp_rank and dp_degree, which are used for | ||
| # dataloader must be changed. | ||
| if self.ft_manager.enabled: | ||
| dp_degree, dp_rank = self.ft_manager.get_dp_info(dp_degree, dp_rank) |
There was a problem hiding this comment.
Similar comment here.. personally I would leave this out for the initial version (again, lmk if you disagree)
| self.tokenizer = tokenizer = ( | ||
| self.train_spec.build_tokenizer_fn(job_config) | ||
| if self.train_spec.build_tokenizer_fn is not None | ||
| else None | ||
| ) |
There was a problem hiding this comment.
This should be done downstream, right?
There was a problem hiding this comment.
discussed offline, probably makes more sense to keep for now
| if job_config.checkpoint.create_seed_checkpoint: | ||
| init_device = "cpu" | ||
| buffer_device = None |
There was a problem hiding this comment.
I'm not sure I understand the purposes of this. Is the idea to do this in lieu of a separate checkpoint conversion script?
There was a problem hiding this comment.
This is more for debugging now. I can remove
| @@ -0,0 +1,351 @@ | |||
| # Copyright (c) Meta Platforms, Inc. and affiliates. | |||
There was a problem hiding this comment.
Skipping over reviewing this file since I assume it's meant more as a demonstration and not really intended to be used directly. Lmk if it warrants a closer look though
There was a problem hiding this comment.
you are right, no need to review too thoroughly for this
depending on #1384