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

Workspace fixes #4541

Merged
merged 10 commits into from Jan 30, 2018

Conversation

Projects
None yet
2 participants
@AlexDBlack
Copy link
Member

AlexDBlack commented Jan 24, 2018

Ready for review.

Fixes: #4542
Fixes: #4539
Fixes: #4533

Also makes the feedForward with clearInputs=false methods more useful / robust.

@AlexDBlack AlexDBlack force-pushed the ab_4539_cg_ext_errors branch from 16be4a9 to da09551 Jan 29, 2018

AlexDBlack added some commits Jan 29, 2018

@AlexDBlack AlexDBlack requested a review from raver119 Jan 29, 2018

@raver119
Copy link
Contributor

raver119 left a comment

Few minor questions, the rest looks great.

case NONE:
workspace = new DummyWorkspace();
break;
case SINGLE:
workspace = Nd4j.getWorkspaceManager().getWorkspaceForCurrentThread(workspaceExternal);
workspace = Nd4j.getWorkspaceManager().getWorkspaceForCurrentThread(workspaceConfigurationExternal, workspaceExternal);

This comment has been minimized.

@raver119

raver119 Jan 30, 2018

Contributor

nice catch

//If using SINGLE mode, workspaceExternal *is* active but then leverageTo becomes a no-op
//and hence we aren't actually moving it out of the current workspace. Consequently, the
//epsilons will be invalidated at the end of the current vertex for loop
epsilons[x] = epsilons[x].detach();

This comment has been minimized.

@raver119

raver119 Jan 30, 2018

Contributor

i'm still not sure why you're detaching it here. In case of SINGLE workspace mode, memory within survives full iteration over 1 dataset

workspace = new DummyWorkspace();
break;
case SINGLE:
workspace = Nd4j.getWorkspaceManager().getWorkspaceForCurrentThread(workspaceExternal);

This comment has been minimized.

@raver119

raver119 Jan 30, 2018

Contributor

workspaceConfigurationExternal is missing

//For single mode, we can't simply leverage to workspaceExternal, as this will currently
// be active. Consequently, the leverage would be a no-op, and hence the epsilons array
//would be invalidated at the end of the current for loop iteration
currPair.setSecond(currPair.getSecond().detach());

This comment has been minimized.

@raver119

raver119 Jan 30, 2018

Contributor

same q as above

@AlexDBlack AlexDBlack merged commit ed7b925 into master Jan 30, 2018

0 of 2 checks passed

codeclimate 5 issues to fix
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@AlexDBlack AlexDBlack deleted the ab_4539_cg_ext_errors branch Jan 30, 2018

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