Skip to content

Conversation

@Xin999
Copy link

@Xin999 Xin999 commented Oct 16, 2017

No description provided.

huzza and others added 30 commits May 11, 2017 22:56
enable rebin capability in stats step
…xception: Noticed that container memory is 2048, while java process consumes 1800+128, I guess there may be some off-heap memory used. Use 1700 m to add 100 m buffer
Conflicts:
	src/main/java/ml/shifu/shifu/core/processor/ExportModelProcessor.java
# Conflicts:
#	CHANGES.txt
#	pom.xml
fix the iv bug in stats and flatten categorical values in when generating GBT tree
trim categorical value whose length is larger than 16k
Fix two bugs in NS supporting and PMML exporting
change categorical group variable delimiter from ^ to @^
@Xin999
Copy link
Author

Xin999 commented Oct 16, 2017

AdaDelta & AdaGrad

/**
*
*/
package ml.shifu.shifu.core.dtrain;
Copy link
Contributor

Choose a reason for hiding this comment

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

import org.slf4j.LoggerFactory;

/**
* @author xchen7
Copy link
Contributor

Choose a reason for hiding this comment

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

No class comment?

if (LOGGER.isDebugEnabled()) {
LOGGER.debug("Initialized with decay " + decay + " and epsilon " + epsilon + " and learning rate "
+ learningRate);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

here is SLF4j, no need wrappered by LOGGER.isDebugEnabled()


/* Update gradients after computation. */
prevGradients = currGradients;

Copy link
Contributor

Choose a reason for hiding this comment

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

the logic looks good, while not sure if master is done, prevGradients will not be recovered, then what will happen? Just image Master is done in iteration 10, then it is recovered to iteration 10, but prevGradients is empty. Other implementations may have the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants