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 relation between IsSavedModel and isFrozen in DnnRetrainTransformer.SaveModel #4197

Merged
merged 1 commit into from
Oct 3, 2019

Conversation

yamachu
Copy link
Contributor

@yamachu yamachu commented Sep 10, 2019

Fix: #4191

IsSavedModel returns true when loaded model is an-frozen model

internal static bool IsSavedModel(IHostEnvironment env, string modelPath)
{
Contracts.Check(env != null, nameof(env));
env.CheckNonWhiteSpace(modelPath, nameof(modelPath));
FileAttributes attr = File.GetAttributes(modelPath);
return attr.HasFlag(FileAttributes.Directory);
}

but now isFrozen variable is set true in spite of an-frozen model.

var isFrozen = DnnUtils.IsSavedModel(_env, _modelLocation);


Checks

  • There's a descriptive title that will make sense to other developers some time from now.
  • There's associated issues. All PR's should have issue(s) associated - unless a trivial self-evident change such as fixing a typo. You can use the format Fixes #nnnn in your description to cause GitHub to automatically close the issue(s) when your PR is merged.
  • Your change description explains what the change does, why you chose your approach, and anything else that reviewers should know.
  • You have included any necessary tests in the same PR.

IsSavedModel returns true when loaded model is an-frozen model (ref: https://github.com/dotnet/machinelearning/blob/1503b0aa9cac997cff8b8bc7e2075eb23d61ad81/src/Microsoft.ML.Dnn/DnnUtils.cs#L137 )
but now isFrozen variable is set true in spite of an-frozen model.
@yamachu yamachu requested a review from a team as a code owner September 10, 2019 03:49
@codemzs
Copy link
Member

codemzs commented Sep 10, 2019

Thanks @yamachu ! I recall fixing this bug in other DNN api but probably missed here. Good work investigating and fixing.

@codemzs codemzs merged commit 940598a into dotnet:master Oct 3, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Mar 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DNN Training] Failed save model in Microsoft.ML.Transforms.DnnTransformer.SaveModel
2 participants