[QEff. Finetune]: Added logger and its test cases. #630
Conversation
quic-meetkuma
requested review from
quic-akuruvil,
quic-swatia,
tchawada and
vbaddi
| assert "Rank zero message" in caplog.text | ||
|
|
||
| @patch("QEfficient.finetune.experimental.core.logger.get_rank") | ||
| def test_log_rank_zero_not_zero(self, mock_get_rank, caplog): |
There was a problem hiding this comment.
Is there a typo in the func name?
There was a problem hiding this comment.
I will update it to
test_log_rank_zero_negative_case
|
|
||
| @patch("QEfficient.finetune.experimental.core.logger.get_rank") | ||
| def test_log_rank_zero_not_zero(self, mock_get_rank, caplog): | ||
| """Test that non-rank zero messages are not logged""" |
There was a problem hiding this comment.
Here also, is it non-zero rank messages?
There was a problem hiding this comment.
I'll update the description.
|
Can we also add a small example script as part of documentation may be, which helps with usage examples for the logger. |
| return dist.is_available() and dist.is_initialized() | ||
|
|
||
|
|
||
| def get_rank() -> int: |
There was a problem hiding this comment.
Will this work fine in case of PP + DDP? Currently, we use os.getenv("LOCAL_RANK", 0) to retrieve the rank in QEff.
There was a problem hiding this comment.
When training on multiple clusters of machines, with each having multiple devices, dist.get_rank() --> gives os.environ["RANK"] which is a global value across nodes and devices.
For us that wont be a problem as long as we dont do multi machine training, because for single machine training LOCAL_RANK = RANK.
For sake of clarity, implemented get_local_rank() as well and we will be internally using the same wherever we are intending to refer local rank 0.
The change will be reflected in latest.
I will add some sample commented text in this PR which will give user hint on how to use the logger. Later on add the same in an extended manner to the documentation as well. |
Yes that would be helpful, thanks. |
…s utility code when dealing with distributed training. Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
quic-meetkuma
force-pushed
the
hf_trainer_logger
branch
from
cdc22a8 to
0fb3f03
Compare
Signed-off-by: meetkuma <[email protected]>
Signed-off-by: meetkuma <[email protected]>
|
Discarding this PR in favor of #644 |
4 participants
TODO: Enable test cases via jenkins infra.