Skip to content
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

Transformer Logs: Save to separate file #3553

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

adityagesh
Copy link
Collaborator

@adityagesh adityagesh commented Dec 12, 2024

Set transformer property 'enable_file_logging' to True to save the transformer logs to separate file

Set transformer property 'enable_file_logging' to True
to save the transformer logs to file
@adityagesh adityagesh force-pushed the aditya/transformer_logging branch from 5124496 to 1bb0db2 Compare December 12, 2024 09:25
@squirrelsc
Copy link
Member

Is this PR to save transformer logs in a separated file, or the transformer logs are not in the whole log file?

@adityagesh
Copy link
Collaborator Author

Is this PR to save transformer logs in a separated file, or the transformer logs are not in the whole log file?

It is to save to a separate file. Corrected the title.

@adityagesh adityagesh changed the title Transformer Logs: Save to file Transformer Logs: Save to separate file Dec 13, 2024
@squirrelsc
Copy link
Member

Is this PR to save transformer logs in a separated file, or the transformer logs are not in the whole log file?

It is to save to a separate file. Corrected the title.

Why does it need to save to separated files? Which scenario does it need? The current env/test split log is easy to understand, but transformer is a LISA concept. It would be confusing to have separated log.

@adityagesh
Copy link
Collaborator Author

adityagesh commented Dec 16, 2024

Is this PR to save transformer logs in a separated file, or the transformer logs are not in the whole log file?

It is to save to a separate file. Corrected the title.

Why does it need to save to separated files? Which scenario does it need? The current env/test split log is easy to understand, but transformer is a LISA concept. It would be confusing to have separated log.

The primary motivation for this PR was the usage in kernel installer. I thought it makes sense to write the kernel installer log to a separate file. Alternatively, we can update the kernel installer transformer to write logs to a file, but I felt having a generic logging solution is better, in case others have similar use cases for different transformer

@squirrelsc
Copy link
Member

The primary motivation for this PR was the usage in kernel installer. I thought it makes sense to write the kernel installer log to a separate file. Alternatively, we can update the kernel installer transformer to write logs to a file, but I felt having a generic logging solution is better, in case others have similar use cases for different transformer

Thank you for telling where the separated log file is used. But I cannot understand why it's needed. In the most scenarios, the kernel installer transformer runs early stage with single thread. It should be easy view in the whole log.

@adityagesh
Copy link
Collaborator Author

The primary motivation for this PR was the usage in kernel installer. I thought it makes sense to write the kernel installer log to a separate file. Alternatively, we can update the kernel installer transformer to write logs to a file, but I felt having a generic logging solution is better, in case others have similar use cases for different transformer

Thank you for telling where the separated log file is used. But I cannot understand why it's needed. In the most scenarios, the kernel installer transformer runs early stage with single thread. It should be easy view in the whole log.

When we want to process the log (eg: using AI) or upload the log (eg: upload the log of kernel build), it is difficult to read from the full lisa log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants