Skip to content

Fix deserialization on Windows #32

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.

Sign up for GitHub

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

Open
GrimMaple wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix deserialization on Windows #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6949168
Fix deserialization on Windows
GrimMaple Apr 23, 2020
e560c09
Prevent writes
GrimMaple May 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/vectorflow/neuralnet.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ class NeuralNet {

if(!_ever_initialized)
{
writeln("Net not initialized. Initializing all weights to 0.");
version(VectorflowPrint) writeln("Net not initialized. Initializing all weights to 0.");
initialize(0.0);
}
{
Expand All @@ -458,7 +458,7 @@ class NeuralNet {

auto cores_str = (
num_cores == 1 ? "1 core." : "%d cores.".format(num_cores));
writeln("Training net with ", num_params, " parameters on ", cores_str);
version(VectorflowPrint) writeln("Training net with ", num_params, " parameters on ", cores_str);
foreach(l; layers)
l.pre_learning();
opt.learn(this, data, grad_f, verbose, num_cores);
Expand Down Expand Up @@ -597,12 +597,12 @@ class NeuralNet {
f.close();
try
{
writeln("Serialization failed.");
version(VectorflowPrint) writeln("Serialization failed.");
remove(path);
}
catch(FileException e)
{
writeln("Couldn't cleanup `", path,
version(VectorflowPrint) writeln("Couldn't cleanup `", path,
"` after serialization failure: ", e);
}
}
Expand Down
32 changes: 25 additions & 7 deletions src/vectorflow/serde.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,32 @@ class Serializer {
final T read(T)() if(isSomeString!T || isBasicType!T || isUnsigned!T)
{
scope(failure){ if(_f.eof) throw new EOFException("");}
scope(exit){ if(_f.eof) throw new EOFException("");}
version(Windows){} // Windows requires special treatment with EOF
else scope(exit){ if(_f.eof) throw new EOFException(""); }
static if(isSomeString!T)
{
ulong str_sz;
_f.rawRead((&str_sz)[0..1]);
auto str = new ubyte[str_sz.to!size_t];
str = _f.rawRead(str);
return cast(string)str;
// On Windows EOF is not set before you try to read past the end, so it requires some special handling
Copy link
Contributor

@CyberShadow CyberShadow Mar 23, 2021

Choose a reason for hiding this comment

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

Is this actually Windows specific? On Linux I get an assertion failure too, with this program:

import std.file;
import std.stdio;

void main()
{
	std.file.write("test.txt", "Test\n");

	auto f = File("test.txt");
	f.readln();
	assert(f.eof);
}

Copy link
Contributor Author

@GrimMaple GrimMaple Mar 30, 2021

Choose a reason for hiding this comment

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

To be fair, I didn't do much checking - the tests failed on Windows and they didn't fail on Linux. Therefore I debugged a little and found out this. I can't remember the details anymore

Copy link
Contributor

@CyberShadow CyberShadow Mar 30, 2021

Choose a reason for hiding this comment

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

This really looks like it needs to be fixed somewhere else.

// Original code was causing runtime crashes rendering deserialization impossible.
// If we use more low level approach and manually throw EOFException if we can't read elemnts it works.
version(Windows)
{
import core.stdc.stdio : fread;
ulong str_sz;
auto sz = fread(&str_sz, str_sz.sizeof, 1, _f.getFP);
if(sz < 1) throw new EOFException("");
auto str = new ubyte[str_sz.to!size_t];
sz = fread(str.ptr, ubyte.sizeof, str_sz, _f.getFP);
if(sz < str_sz) throw new EOFException("");
return cast(string)str;
}
else
{
ulong str_sz;
_f.rawRead((&str_sz)[0..1]);
auto str = new ubyte[str_sz.to!size_t];
str = _f.rawRead(str);
return cast(string)str;
}
}
else
{
Expand Down Expand Up @@ -99,7 +117,7 @@ class Serializer {
auto l = Object.factory(layer_type).to!NeuralLayer;
l.deser(this);
layers ~= l;
writeln("Deserialized ", l.to!string);
version(VectorflowPrint) writeln("Deserialized ", l.to!string);
}

return layers;
Expand Down
6 changes: 3 additions & 3 deletions test/common.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,12 @@ version(unittest)
}
}
}
writeln("----------");
version(VectorflowPrint) writeln("----------");
if(failed > 0)
{
writefln("At least %d test(s) failed.", failed);
version(VectorflowPrint) writefln("At least %d test(s) failed.", failed);
foreach(m; retro(messages))
writeln(m);
version(VectorflowPrint) writeln(m);
}
return failed == 0;
}
Expand Down