Add thousand-separator commas to TotalParams #7910
Conversation
The number of parameters can be quite large, and it would help the reading of the summary printout to have the TotalParams column & values at the bottom have thousand-separator-commas in them.
There was a problem hiding this comment.
LGTM. But for consistency, let's make the same change to MultiLayerNetwork?
https://github.com/eclipse/deeplearning4j/blob/master/deeplearning4j/deeplearning4j-nn/src/main/java/org/deeplearning4j/nn/multilayer/MultiLayerNetwork.java#L3642-L3646
Corresponding change to MultiLayerNetwork
|
Added the MultiLayerNetwork change you requested. Signed the Eclipse Contributor Agreement, but it's still complaining about some Signed-off-by footer, which I don't know how to fix. |
|
@jxtps I think that just means you need to do your commits with |
|
I'll redo it. |
|
Redoing it failed. Sorry, but I give up. I just wanted to do a quick edit I thought would be useful in the Github web interface - I haven't even checked out the code to my desktop. Too many hoops for too little benefit. |
|
Doing |
|
Sure, but those don't seem to be available in the github web interface? Yes, of course I could check out the code to my desktop, jump through whatever hoops to get things working on this account, etc, etc. But. For super-tiny-but-useful changes like this, not to mention documentation changes, requiring that additional workflow is very frustrating. There is something magically powerful about being able to navigate to a file on github.com click the little pen icon, make some edits, done. Signing the ECA isn't that big of a deal since it's a one-off, but the signed-off-by footer... sigh... the lawyers won and everyone else lost. Maybe Github could add the signed off footer feature to their online commits - should basically "just" be a checkbox. Ok, I'm going to stop whining now ;) |
|
The web UI, yeah ok, looks like we got an issue open for that here: todogroup/gh-issues#50 |
What changes were proposed in this pull request?
The number of parameters can be quite large, and it would help the reading of the summary printout to have the TotalParams column & values at the bottom have thousand-separator-commas in them. This PR adds that.
How was this patch tested?
Sorry, no testing, edits are tiny and hopefully clear. Didn't run mvn formatter since all changes were inline.