Skip to content

Conversation

@suo
Copy link
Member

@suo suo commented May 31, 2019

Stack from ghstack:

Don't write the same file multiple times. This doesn't have any
observable effect on model loading, but it makes running unzip manually
confusing (since it will try to unzip multiple files with the same name
and ask whether to overwrite them).

Differential Revision: D15581527

Don't write the same file multiple times. This doesn't have any
observable effect on model loading, but it makes running unzip manually
confusing (since it will try to unzip multiple files with the same name
and ask whether to overwrite them).
@pytorchbot pytorchbot added the oncall: jit Add this issue/PR to JIT oncall triage queue label May 31, 2019
@suo suo requested review from driazati, jamesr66a and zdevito May 31, 2019 03:39
const ClassTypePtr& class_type = item.key();
const std::string filename =
ImportExportHelpers::qualifierToPath(class_type->qualifier());
if (written_files.count(filename)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why would it be trying to write out the same class_type multiple times in the first place?

Copy link
Member Author

Choose a reason for hiding this comment

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

Multiple class types can resolve to the same file.

@suo suo requested a review from driazati May 31, 2019 21:19
Copy link
Contributor

@driazati driazati left a comment

Choose a reason for hiding this comment

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

lgtm, is this check still needed?

if (imported_libs_.count(qualifier)) {

@zdevito zdevito removed their request for review June 1, 2019 01:40
@zou3519 zou3519 deleted the gh/suo/44/head branch June 1, 2019 02:17
@facebook-github-bot
Copy link
Contributor

@suo merged this pull request in c8dc707.

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

Labels

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.

6 participants