Skip to content

CoreML backend should log on failure not success#82604

Closed
jmdetloff wants to merge 1 commit intopytorch:masterfrom
jmdetloff:johndetloff/logging_fix
Closed

CoreML backend should log on failure not success#82604
jmdetloff wants to merge 1 commit intopytorch:masterfrom
jmdetloff:johndetloff/logging_fix

Conversation

@jmdetloff
Copy link
Contributor

Description

The original intention of this code was to log whenever inference fails, not whenever it succeeds. This change updates the logic of should_log to match this intention.

@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Aug 1, 2022

🔗 Helpful links

✅ No Failures (0 Pending)

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

Expand to see more

💚 💚 Looks good so far! There are no failures yet. 💚 💚


This comment was automatically generated by Dr. CI (expand for details).

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 Aug 1, 2022
@jmdetloff jmdetloff requested a review from mcr229 August 1, 2022 20:50
@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@pytorchbot merge

(Initiating merge automatically since Phabricator Diff has merged)

@pytorchmergebot
Copy link
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2022

Hey @jmdetloff.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

facebook-github-bot pushed a commit that referenced this pull request Aug 2, 2022
Summary:
### Description
The original intention of this code was to log whenever inference fails, not whenever it succeeds. This change updates the logic of should_log to match this intention.

Pull Request resolved: #82604

Test Plan: One line log fix. Build, run tests, and lint.

Reviewed By: SS-JIA, kirklandsign

Differential Revision: D38327492

Pulled By: jmdetloff

fbshipit-source-id: 710a59670a76b6f756501cc97373e81a955f5328
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants