Skip to content

Exception loading an older model file with no strings #1289

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

Closed
eerhardt opened this issue Oct 17, 2018 · 0 comments
Closed

Exception loading an older model file with no strings #1289

eerhardt opened this issue Oct 17, 2018 · 0 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@eerhardt
Copy link
Member

When I changed our model format for the ComponentCatalog changes (#970), I refactored some model loading code incorrectly. There are some existing models that are breaking our validation checks:

The old code was doing:

if (header.FpStringTable == 0)
{
// No strings.
strings = null;
ex = null;
return true;

The new code now does:

if (header.FpStringTable == 0)
{
// No strings.
strings = null;
}

Notice that it doesn't return early in the new code. Lower in the method there are checks here:

Contracts.CheckDecode(header.FpTail == reader.FpCur() - fpMin);
ulong tail = reader.ReadUInt64();
Contracts.CheckDecode(tail == TailSignatureValue, "Corrupt model file tail");

These checks are now failing with older model files. They were never run in the old code when there were no strings, so we shouldn't be running them anymore when reading older model files.

/cc @yaeldekel

@eerhardt eerhardt self-assigned this Oct 17, 2018
@eerhardt eerhardt added the bug Something isn't working label Oct 17, 2018
eerhardt added a commit to eerhardt/machinelearning that referenced this issue Oct 17, 2018
Older model files are failing to load due to FpTail checks in the model validation code. These checks weren't happening in the old model loading code, and they now fail on some older models.

Fix this by returning early when it is an older model, and there are no strings. This preserves the old behavior.

Fix dotnet#1289
eerhardt added a commit to eerhardt/machinelearning that referenced this issue Oct 18, 2018
Older model files are failing to load due to FpTail checks in the model validation code. These checks weren't happening in the old model loading code, and they now fail on some older models.

Fix this by returning early when it is an older model, and there are no strings. This preserves the old behavior.

Fix dotnet#1289
@shauheen shauheen added this to the 1018 milestone Oct 18, 2018
eerhardt added a commit that referenced this issue Oct 18, 2018
* Loading old model files is broken.

Older model files are failing to load due to FpTail checks in the model validation code. These checks weren't happening in the old model loading code, and they now fail on some older models.

Fix this by returning early when it is an older model, and there are no strings. This preserves the old behavior.

Fix #1289
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants