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
Added Neural HardTanh Layer and unit test #3037
Conversation
virtual const char* get_name() const { return "NeuralHardTanhLayer"; } | ||
|
||
protected: | ||
/** Parameter used to calculate max_val(min_val*(W*x+b),W*x+b). |
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.
Can you document both parameters seperately?
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.
Done. I also changed the names from min_val and max_val to min_act and max_act. Thought it made more sense.
Very nice and polished PR! Fine to merge from my side apart from the minor things I commented. |
7805262
to
16f9a36
Compare
A_ref(i,j) += weights[i+k*A_ref.num_rows]*x(k,j); | ||
|
||
A_ref(i,j) = A_ref(i,j) >= max_act ? max_act : | ||
CMath::max<float64_t>(A_ref(i,j),min_act); |
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 many loops .... what about some eigen3/linalg dot products and or openmp?
@karlnapf The third commit should've handled all your comments... expect the linalg/openmp one. @vigsterkr @karlnapf @lisitsyn To be honest, the entire neuralnets package is filled with for loops and (afaik) doesn't use the linalg library much, either. I think it can all be vectorized and refactored with openmp/shogun's linalg. What do you guys think? Thanks. :) |
@arasuarun the neuralnet part of shogun was written when the linalg package was starting to emerge. no wonder that it doesn't use all of the linalg backend. |
CNeuralLinearLayer::compute_activations(parameters, layers); | ||
|
||
int32_t len = m_num_neurons*m_batch_size; | ||
for (int32_t i=0; i<len; i++) |
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.
parallelize
About loops. I think we can merge this before putting in openmp. Openmp would be a next step. I think this should be done globally in the NN package (spend some time thinking about the design), and for sure a seperate PR. Agree @vigsterkr ? |
Would be a super useful piece of work to parallelise/linalg everything in NNs.... |
why to do things tomorrow when you can do it today? :) |
I'll start by fixing the Tanh and HardTanh layers with linalg and openmp. I'll do so by by the end of this weekend after learning Shogun's openmp conventions and its linalg lib. |
@vigsterkr @karlnapf I'm not able to understand one thing: how does Shogun get its GPU support? Afaik, OpenMP isn't designed for GPUs... right? Although I did read that omp 4.0 does codgen for Nvidia GPUs. |
@arasuarun noup openmp is not for gpu: https://en.wikipedia.org/wiki/OpenMP |
Oh! ViennaCL. Got it. |
Gpu support comes from ViennaCL.
|
news? |
Sure totally fine. |
A HardTanh layer could be a nice addition to the neural nets package. It is viewed as an approximation of Tanh:
The range can be set by the user. Default is (-1, 1).
Not sure of its exact usage (maybe rnn, lstm), but it's present in other dnn frameworks (like torch).
(Edit:) Some sources: