close
The Wayback Machine - https://web.archive.org/web/20201207013518/https://github.com/intel-analytics/BigDL/pull/2368
Skip to content
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

Model Visualization enabled - Resnet & Inception #2368

Open
wants to merge 5 commits into
base: master
from

Conversation

@SnehalA
Copy link

@SnehalA SnehalA commented Mar 8, 2018

What changes were proposed in this pull request?

-Enabled Graph topology, training summaries in tensorboard for Resnet and inception models.
-Added parameter "visualization"
-Added Top5 accuracy capture for Resnet

(Please fill in changes proposed in this patch)

How was this patch tested?

Manual tests
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If it is possible, please attach a screenshot; otherwise, remove this)

Related links or issues (optional)

fixed #2310

SnehalA added 5 commits Feb 28, 2018
Copy link
Contributor

@yiheng yiheng left a comment

Hi, I think this is a very useful feature. Thanks for the contribution!

I have some suggestion in my comments. Please check and welcome discussion.

@@ -39,7 +39,8 @@ object Options {
warmupEpoch: Option[Int] = None,
gradientL2NormThreshold: Option[Double] = None,
gradientMin: Option[Double] = None,
gradientMax: Option[Double] = None
gradientMax: Option[Double] = None,
visualization: Boolean = true

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

Can we change the visualization:Boolean to something like summary:Option[String] and let user input the visualization summary location? So if user inputs such value, we can save the graph visualization file in that location instead of hard code it in our code; if user doesn't input, it means he/she doesn't want to enable the visualization

@@ -39,7 +39,8 @@ object Options {
warmupEpoch: Option[Int] = None,
gradientL2NormThreshold: Option[Double] = None,
gradientMin: Option[Double] = None,
gradientMax: Option[Double] = None
gradientMax: Option[Double] = None,
visualization: Boolean = true

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

Also please update the ReadMe.md file in the same folder to let user know how to use this new option.

@@ -39,7 +39,8 @@ object Options {
warmupEpoch: Option[Int] = None,
gradientL2NormThreshold: Option[Double] = None,
gradientMin: Option[Double] = None,
gradientMax: Option[Double] = None
gradientMax: Option[Double] = None,
visualization: Boolean = true

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

I think you may miss the change in the trainParser?

@@ -118,6 +120,19 @@ object TrainInceptionV1 {
if (param.gradientL2NormThreshold.isDefined) {
optimizer.setGradientClippingByl2Norm(param.gradientL2NormThreshold.get.toFloat)
}

if (param.visualization) {
val logdir = "bigdlSummary"

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

As mentioned above, can you let user input the logdir instead of hard code one in the code


if (param.visualization) {
val logdir = "bigdlSummary"
val timestamp: Long = System.currentTimeMillis / 1000

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

can we convert the timestamp into a more human readable format? I have a similiar change in my local. I use the human readable time as the suffix. For your reference

val sdf = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss")
val timeStamp = sdf.format(new Date())
optimizer.setValidationSummary(validationSummary)
optimizer.setTrainSummary(trainSummary)
model.asInstanceOf[Graph[Float]].saveGraphTopology(logdir)
//print("class Num "+param.classNumber)

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

please remove this commented line

@@ -89,11 +91,23 @@ object Train {
if (param.checkpoint.isDefined) {
optimizer.setCheckpoint(param.checkpoint.get, Trigger.everyEpoch)
}

if (param.visualization) {
val logdir = "bigdlSummary"

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

Please check my comments in the above

@@ -42,7 +42,8 @@ object Utils {
momentum: Double = 0.9,
dampening: Double = 0.0,
nesterov: Boolean = true,
graphModel: Boolean = false)
graphModel: Boolean = false,
visualization: Boolean = true)

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

please check my comments in the inception code

trainSummary.setSummaryTrigger("Parameters", Trigger.severalIteration(10))
optimizer.setValidationSummary(validationSummary)
optimizer.setTrainSummary(trainSummary)
model.asInstanceOf[Graph[Float]].saveGraphTopology(logdir)

This comment has been minimized.

@yiheng

yiheng Mar 9, 2018
Contributor

we should check if this is real a graph model.

if (model.isInstanceOf[Graph[Float]]) ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.