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
IRV update #1054
IRV update #1054
Conversation
out_tensor = tf.nn.l2_loss(self.IRVLayer.W) + \ | ||
tf.nn.l2_loss(self.IRVLayer.V) + tf.nn.l2_loss(self.IRVLayer.b) + \ | ||
tf.nn.l2_loss(self.IRVLayer.b2) | ||
out_tensor = out_tensor * self.penalty |
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.
IRV has l2 regularization, is there a better way to handle these extra loss values?
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 this looks fine.
out_tensor = tf.nn.l2_loss(self.IRVLayer.W) + \ | ||
tf.nn.l2_loss(self.IRVLayer.V) + tf.nn.l2_loss(self.IRVLayer.b) + \ | ||
tf.nn.l2_loss(self.IRVLayer.b2) | ||
out_tensor = out_tensor * self.penalty |
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 this looks fine.
@@ -1184,6 +1184,26 @@ def create_tensor(self, in_layers=None, set_tensors=True, **kwargs): | |||
return out_tensor | |||
|
|||
|
|||
class Sigmoid(Layer): |
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.
Could you add a docstring for this class along with a usage example?
|
||
|
||
class Slice(Layer): | ||
""" Choose a slice of input given axis and order |
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.
Could you add a usage example for this class in the docstring?
@@ -1531,6 +1551,27 @@ def create_tensor(self, in_layers=None, set_tensors=True, **kwargs): | |||
return out_tensor | |||
|
|||
|
|||
class SigmoidCrossEntropy(Layer): |
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.
Could you add a usage example and docstring for this class?
|
||
|
||
class TensorflowMultiTaskIRVClassifier(TensorflowLogisticRegression): | ||
class IRVLayer(Layer): |
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.
Could you add a unit test for these and the other layers added?
@miaecle Thanks for porting! Made a few comments with requests for more docs and tests. |
@rbharath Thanks for the review! Just added some more docstrings and unit tests, let me know if anything else could be improved. |
@rbharath No problem, I will handle it. |
Thanks for the fixes! LGTM |
This PR adds in:
Tensorgraph version of IRV with identical structure
TensorflowLogisticRegression is removed and standard sklearn logistic regression is used in examples and benchmark