-
Notifications
You must be signed in to change notification settings - Fork 3
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
defModule macro to define neural nets without inline C++ #5
Comments
Thus far I've got an untyped macro that can generate the emitted C++ code and add the correct pragmas. Would it make sense to also generate a flambeau/examples/pytorch/cpp/mnist/mnist.nim Lines 24 to 47 in cba9cc5
We could use your syntax from above (if we assume all types are initialized with nullptr ) and write it like this:
defModule:
type Net = object of Module
conv1: Conv2d = (1, 10, 5)
conv2: Conv2d = (10, 20, 5)
conv2_drop: Dropout2d # no value means passing no parameters
fc1: Linear = (320, 50)
fc2: Linear = (50, 10) Then the values would be passed to the type's I'm not too used to Torch yet so I'm not really sure whether this is an approach that would work in general for a typical Net, so any feedback is appreciated. |
I think something like this is better: defModule:
type Net = object of Module
conv1 = Conv2d(1, 10, 5)
conv2 = Conv2d(10, 20, 5)
conv2_drop: Dropout2d # no value means passing no parameters
fc1 = Linear(320, 50)
fc2 = Linear(50, 10) It avoids floating untyped parenthesis and mimics object construction. The tricky parts are:
|
That does indeed look more intuitive. 👍 And with the power of pattern matching not that much harder either 😎
And another thing that I think got lost yesterday, should we support Nim types in a Net? I don't really see how we could include them in the emitted C++ types so I'd say: no 😛 Plus we have no way of telling if it is a Nim type or C++ type in a untyped network. |
Think I realized what you meant by nested nets now, something along these lines, right? defModule:
type
Net = object of Module
conv1 = Conv2d(1, 10, 5)
conv2 = Conv2d(10, 20, 5)
conv2_drop: Dropout2d # no value means passing no parameters
fc1 = Linear(320, 50)
fc2 = Linear(50, 10)
Net2 = object of Module
net1: Net1
fc = Linear(100, 100) If we go for an untyped macro we'd need a separate syntax for nested nets. Perhaps we could do it like this, that custom Nets are specified in "Nim-style" ( Net2 = object of Module
net1: Net1
fc = Linear(100, 100) This would be easy to parse at least. But the again, it depends on what types we should allow in a Net. |
Why not just do: defModule:
type
Net = object of Module
conv1 = Conv2d(1, 10, 5)
conv2 = Conv2d(10, 20, 5)
conv2_drop: Dropout2d # no value means passing no parameters
fc1 = Linear(320, 50)
fc2 = Linear(50, 10)
Net2 = object of Module
net1 = Net()
fc = Linear(100, 100) in those cases? |
@Vindaar Because we need to be able to differentiate between them when generating the emitted C++. The code for struct Net2: public torch::nn::Module {
Net net1{nullptr}; // here's the difference, no namespace
torch::nn::Linear fc{nullptr};
}; But the generated code will be this if we can't see a difference between custom Nets (that we generate the C++ code for) and the built-in ones: struct Net2: public torch::nn::Module {
torch::nn::Net net1{nullptr}; // here's the difference, wrong namespace
torch::nn::Linear fc{nullptr};
}; Built-in layers like |
Ah, but that can be easily solved by keeping track of all nets that were user generated (e.g. |
Yes, that should work but then we're back at how to do that safely between macros. As people could use multiple edit: This approach also makes the C++ generation more complex as it must first parse all the code, and then create all of the C++ structs. The current approach just creates the C++ code while parsing which is more simple. With different syntaxes that could still be the case, but that's just me being lazy 🤣 |
Oh ok, now I'm starting to see what you're talking about. A shared CT table is something that should be supported anyways, imo. So it's only a matter of time until that is / should be implemented. Maybe one of us can help out improving the With the fully untyped, no state approach, another option might be something like wrapping the name in a call, e.g.: defModule:
type
Net = object of Module
conv1 = Conv2d(1, 10, 5)
conv2 = Conv2d(10, 20, 5)
conv2_drop: Dropout2d # no value means passing no parameters
fc1 = Linear(320, 50)
fc2 = Linear(50, 10)
Net2 = object of Module
net1 = custom(Net())
fc = Linear(100, 100) e.g. |
Yeah, it definitely should be supported, and we probably should help improve it. But it feels like it's quite convoluted to handle everything in an IC-safe way without digging our fingers into how the internals of IC works 🤔 That approach is more feasible and future-proof in my opinion 😄 And you have a good point in that it is easier to document and understand than different syntaxes. And the hardest part is of course finding a good name for it 🤣 but |
Mnist now works without having to manually emit the C++ type and the struct Net: public torch::nn::Module {
torch::nn::Conv2d conv1{nullptr};
torch::nn::Conv2d conv2{nullptr};
torch::nn::Dropout2d conv2_drop{nullptr};
torch::nn::Linear fc1{nullptr};
torch::nn::Linear fc2{nullptr};
};
struct Net2: public torch::nn::Module {
torch::nn::Linear fc1{nullptr};
Net net1{nullptr}; // error here
}; But then I get this error:
It feels like |
Great work! About the issue, maybe this doesn't make any sense, but: To be honest, I don't even know exactly what providing a value within a |
Thanks 😄 That sound very reasonable, the built-in layers are shared_ptrs so making nullptr default for them makes sense but not for us. So the question is what to put in there instead. I had a look here https://pytorch.org/tutorials/advanced/cpp_frontend.html and they seem to define a struct first and then run the |
Something as easy as removing the default value altogether seems to have stopped the compiler from complaining at least: |
Indeed, that does make sense. Now you're just saying "this struct contains a member |
Yeah exactly 👍 |
Personally I would probably try to see how far your current approach can work. It's "only" a macro and we can always rewrite parts of it. :) |
Agreed! Both because it is simpler and because I don't want to start over 🤪 |
Tested it and initializing nested Nets seems to work (tomorrow is another day, namely today :P) Something that doesn't work is defining custom Nets in one file and importing it into another though. And that makes sense as the emitted C++ in the original file isn't visible to the file that imports the types. I guess this is a issue for us? A hack I found to "solve" it is to define the types in a template and importing and running the template. This way all emitted types are visible. Here an example: # nets.nim
template importNets*: untyped {.dirty.} =
defModule:
type
Net* = object of Module
conv1* = Conv2d(1, 10, 5)
conv2* = Conv2d(10, 20, 5)
conv2_drop* = Dropout2d()
fc1* = Linear(320, 50)
fc2* = Linear(50, 10)
Net2* = object of Module
net1* = custom Net()
fc1* = Linear(4, 1) # training.nim
import ./nets
importNets()
var model: Net
model.init() I don't know how we could solve it otherwise unless we can get the emitted C++ code visible across files. It's the C++ compiler that complains in the end that |
Have done some further testing with nesting now and it doesn't seem to work unless we register the subnet and it seems to expect a shared_ptr to a net. So we may have to use TORCH_MODULE after all :/ Edit: realized that the dot macros won't work because the Nim compiler thinks it can access the fields so the macro won't be invoked. :/ |
I'm not sure what you mean by we have to use |
No, I mean when we want to access the fields of our Nets (in the |
Sorry for spamming, just want to get my thoughts down in writing while they are on the top of my head. Here's how it works in C++: struct LinearImpl : torch::nn::Module {
// define the members here
};
TORCH_MODULE(Linear);
type Net {.importcpp.} = object of ModuleHolder
# it has fields conv1, conv2, fc1
# generate the procs for accessing them:
proc conv1*(net: var Net): ModuleHolder {.importcpp: "#->conv1".}
proc conv2*(net: var Net): ModuleHolder {.importcpp: "#->conv2".}
proc fc1*(net: var Net): ModuleHolder {.importcpp: "#->fc1".}
# Now it can be used as an ordinary object:
var net: Net
net.init()
let y = net.conv1.forward(x) Another approach would be to wrap type Net = CppSharedPtr[NetImpl]
var net: Net
net.init()
let y = net.conv1.forward(x) # `.` macro handles it so we generate `->` instead of `.` I haven't implemented any of them yet though so no idea if it could work. edit: Not sure if it was clear but the reason we need the module as a pointer is that |
Here comes a bit more rambling from me. I've thought a bit more about this and I think doing it the type
Module* {.bycopy, pure, inheritable, importcpp: "torch::nn::Module".} = object
## A LibTorch neural network module that can be inherited from
ModuleHolder*[T: Module] {.bycopy, pure, importcpp: "torch::nn:ModuleHolder<'0>".} = object
## A Libtorch wrapper around std::shared_ptr<Module>
type
LinearImpl* {.pure, bycopy, importcpp: "torch::nn::LinearImpl".} = object of Module
options*{.importc.}: LinearOptions
weight*{.importc.}: Tensor
bias*{.importc.}: Tensor
Linear* {.pure, bycopy, importcpp: "torch::nn::Linear".} = ModuleHolder[LinearImpl]
# Linear is a shared_ptr underneath.
# The ptr is bycopy which results in the actual data being byref.
Conv2dImpl* {.pure, bycopy, importcpp: "torch::nn::Conv2dImpl".} = object of Module
options*{.importc.}: Conv2DOptions
weight*{.importc.}: Tensor
bias*{.importc.}: Tensor
Conv2d* {.pure, bycopy, importcpp: "torch::nn::Conv2d".} = ModuleHolder[Conv2dImpl]
# Conv2d is a shared_ptr underneath.
# The ptr is bycopy which results in the actual data being byref. Then we just define a dot macro to handle the dereferencing of a |
Of course, Nim doesn't like it when we use inherited types as generic types 🤣:P Have to find another way to do this it seems. |
Heureka! Think I've got it to work finally 🙃 So I'm leaning more and more towards using |
@mratsim Any reason why you went with var net: Net
net.init()
# instead of
var net: Net = Net.init() The second way is how the builtin layers are initialized so for consistency we could use it for custom Nets as well? |
Ok, now I've got something that works actually works for nested nets as well. Added support for parameters (Tensors) as well. So my final proposal for the syntax is this: defModule:
type
Net* = object of Module
conv1* = Conv2d(1, 10, 5)
conv2* = Conv2d(10, 20, 5)
conv2_drop* = Dropout2d()
fc1* = Linear(320, 50)
fc2* = Linear(50, 10)
Net2 = object of Module
anotherNet = custom Net
w = param randn(100, 100)
bias = param randn(100)
var net = Net.init()
var net2 = Net2.init() The only thing that feels redundant is Now you are free from my spamming here for now 🤣 |
Because some layers might not be copyable and so I want to avoid assignments. |
@mratsim Okay 👍 But if we wrap all of them in |
Currently we need to inline C++ code due to Nim not being able to generate C++ code with default value or wrap types without default constructors nim-lang/Nim#4687
This leads to the following code
flambeau/proof_of_concepts/poc09_end_to_end.nim
Lines 12 to 26 in fe82c68
with emittypes a workaround nim-lang/Nim#16664
flambeau/flambeau/cpp/emitters.nim
Lines 3 to 13 in fe82c68
Instead it would be more convenient to have a macro
defModule
(name subject to bikeshedding) with the following syntax:and generates the proper C++ code (nil mapped to {nullptr}) and rewrite the typesection with the proper {.pure, importcpp.} pragma.
This is similar to nim-lang/RFCs#126, nim-lang/RFCs#252
Implementation details:
{.emit:["type something {", NimTypeInterpolated, " field{nullptr};"].}
The text was updated successfully, but these errors were encountered: