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

Fix graph building to exclude input, output and initializer from value_info #1320

Merged
merged 2 commits into from
Apr 1, 2024

Conversation

BowenBao
Copy link
Contributor

@BowenBao BowenBao commented Mar 29, 2024

Stack from ghstack (oldest at bottom):

Otherwise the emitted model proto instance violates the spec definition for value_info.

BowenBao added a commit that referenced this pull request Mar 29, 2024
…e_info

ghstack-source-id: b9a1745b6d36beba070db773a4d2932288880267
Pull Request resolved: #1320
@BowenBao BowenBao requested a review from justinchuby March 29, 2024 18:19
@@ -140,7 +139,6 @@ def test_add_initializer_allows_adding_the_same_tensor_twice_using_same_name(sel


class TestModelSaving(unittest.TestCase):
@unittest.skipIf(os.getenv("CI") == "true", "CI is not ready to run dyanmo_export.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure about the context here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Initially we disabled it to avoid flakiness due to the changing pytorch

Copy link

github-actions bot commented Mar 29, 2024

Test Results

     18 files  ±     0      18 suites  ±0   1h 7m 43s ⏱️ + 13m 49s
  7 583 tests +     1   5 526 ✅  -      1    2 054 💤  -      1   3 ❌ + 3 
196 467 runs  +50 493  46 127 ✅ +12 685  150 329 💤 +37 797  11 ❌ +11 

For more details on these failures, see this check.

Results for commit 17b53c1. ± Comparison against base commit 2612107.

♻️ This comment has been updated with latest results.

…r from value_info"


Otherwise the emitted model proto instance violates the spec definition for value_info.


[ghstack-poisoned]
BowenBao added a commit that referenced this pull request Mar 29, 2024
…e_info

ghstack-source-id: 4c40be31d6af9e243296bfa3dca08ae4a4a5b3b5
Pull Request resolved: #1320
@justinchuby
Copy link
Collaborator

justinchuby commented Mar 29, 2024

Good to merge. #539

@BowenBao BowenBao merged commit 9418aec into gh/BowenBao/18/base Apr 1, 2024
21 of 33 checks passed
@BowenBao BowenBao deleted the gh/BowenBao/18/head branch April 1, 2024 16:35
@BowenBao
Copy link
Contributor Author

BowenBao commented Apr 1, 2024

Ouch, wrong base again.

justinchuby pushed a commit that referenced this pull request Apr 1, 2024
…e_info (#1321)

Stack from [ghstack](https://github.com/ezyang/ghstack) (oldest at
bottom):
* __->__ #1321

Otherwise the emitted model proto instance violates the spec definition
for value_info.
Reland #1320
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants