Skip to content

Conversation

narang99
Copy link
Contributor

Description:

  • Have only added stdout and stderr as possible options from python
    API for now. We can do file path passing later maybe.
  • Put the class JitLoggingConfig in the cpp file as none of its methods were being used outside of this file.

Python API:
torch._C._jit_set_logging_stream('stdout|stderr')
C++ API:
::torch::jit::set_jit_logging_output_stream(ostream);

Testing:

  • Tested python API locally.
  • Unit test for the C++ API is written

Fixes #54182

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 28, 2021

🔗 Helpful links

💊 CI failures summary and remediations

As of commit ed78826 (more details on the Dr. CI page):


None of the CI failures appear to be your fault 💚



🚧 1 fixed upstream failure:

These were probably caused by upstream breakages that were already fixed.

Please rebase on the viable/strict branch (expand for instructions)

If your commit is older than viable/strict, run these commands:

git fetch https://github.com/pytorch/pytorch viable/strict
git rebase FETCH_HEAD

This comment was automatically generated by Dr. CI (expand for details).Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions to the (internal) Dr. CI Users group.

Click here to manually regenerate this comment.

@facebook-github-bot facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Sep 28, 2021
@narang99
Copy link
Contributor Author

I checked the occurrences of TORCH_API and its being used before the return type for function signatures in other files. So I modified the signatures in this one too to follow the convention

Copy link

@ZolotukhinM ZolotukhinM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@facebook-github-bot
Copy link
Contributor

@ZolotukhinM has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got some internal testing failures, and I think it's because we're missing "::torch::jit" namespace before "get_jit_logging_output_stream". Do you mind adding it?

Description:
- Have only added `stdout` and `stderr` as possible options from python
  API for now. We can do file path passing later maybe.
- Put the class `JitLoggingConfig` in the cpp file as none of its methods were being used outside of this file.

Python API:
`torch._C._jit_set_logging_stream('stdout|stderr')`
C++ API:
`::torch::jit::set_jit_logging_output_stream(ostream);`

Testing:
- Tested python API locally.
- Unit test for the C++ API is written
@facebook-github-bot
Copy link
Contributor

@ZolotukhinM has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

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

Labels

cla signed oncall: jit Add this issue/PR to JIT oncall triage queue open source

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[JIT] Provide an API to choose a different output stream for PYTORCH_JIT_LOG_LEVEL

4 participants