-
Notifications
You must be signed in to change notification settings - Fork 180
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
Support for basic THFile operations. #54
Conversation
Odd -- it's not re-running the checks. |
/// <summary> | ||
/// The storage class provides a mechanism to access the underlying data representation for memory files. | ||
/// </summary> | ||
public class CharStorage : IDisposable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So my understanding is that CharStorage is used here while the other type of storages are used to back a tensor. Is this correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's what the C interface is using for memory files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storages now are in 2 different files. Should we do as in Torch where all storages classes are in one file? This can be solved in a separate PR (related to #50?)
TorchSharp/TorchSharp.csproj
Outdated
</Compile> | ||
</ItemGroup> | ||
<ItemGroup> | ||
<Service Include="{508349b6-6b84-4df5-91f0-309beebad82d}" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can remove this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Service tag?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know why it keeps coming back. I've removed it before.
This seems to be using two different set of indentation styles in P/Invoke declarations. |
@migueldeicaza, are you referring to the placement of the [DllImport] attribute? The one-line format came about because I copied code from C, and then did a global replace. The latest update should have this addressed, with two lines for each declaration. If you're referring to something else, please explain. |
/// <summary> | ||
/// The storage class provides a mechanism to access the underlying data representation for memory files. | ||
/// </summary> | ||
public class CharStorage : IDisposable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storages now are in 2 different files. Should we do as in Torch where all storages classes are in one file? This can be solved in a separate PR (related to #50?)
Are we good with this PR now? |
With this PR, support for MemoryFile and DiskFile is introduced.