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

Correct path and update version for ComputeCpp #32

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

krikru
Copy link
Contributor

@krikru krikru commented Jan 19, 2017

This commit will:

  • Correct path to bin folder for ComputeCpp

  • Update ComputeCpp version from 0.1.1 to 0.1.2

Luke Iwanski and others added 4 commits December 22, 2016 08:26
Added Tile, Transpose and Range Ops double support for SYCL device.
Moved gpu_device_name() to test_util.py so now it can be used in force_gpu to pull either GPU or SYCL depending on what is available in the system.
 - Registration of Type Traits required for stride slice op
 - Registration of ConcatOffset, _ListToArray, _ArrayToList
   Pad, Reverse ( CPU ), ReverseV2 ( CPU ), Size, ExpandDims,
   Squeeze, StridedSlice, StridedSliceGrad, StridedSliceAssign,
   TileGrad, InvertPermutation, Transpose
 - Registration of Sycl kernels only for essential data types
 - Floor_div_real has been disabled for SYCL device
 - Device in control_flow_ops_py_test.py needed to be lower cased
Conflicts:
	tensorflow/core/kernels/strided_slice_op_impl.h
	tensorflow/python/platform/test.py
This commit will:

* Correct path to bin folder for ComputeCpp

* Update ComputeCpp version from 0.1.1 to 0.1.2
sudo chmod -R a+r /usr/local/computecpp/
sudo chmod -R a+x /usr/local/computecpp/bin
sudo chmod -R a+x /usr/local/computecpp/ComputeCpp-CE-0.1.2-Linux/bin
Copy link
Owner

Choose a reason for hiding this comment

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

We want to make the binaries we copied in from ComputeCpp-CE-0.1.2-Linux/bin to /usr/local/computecpp/bin executable. I think that the original path was therefore correct

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I run

sudo chmod -R a+x /usr/local/computecpp/bin

I get

chmod: cannot access ‘/usr/local/computecpp/bin/’: No such file or directory

because the line

sudo cp -R ComputeCpp-CE-0.1.2-Linux /usr/local/computecpp

seems to copy the whole folder to /usr/local/computecpp, not just the folder contents (i.e. when I run ls /usr/local/computecpp I get ComputeCpp-CE-0.1.2-Linux, and the bin folder ends up in /usr/local/computecpp/ComputeCpp-CE-0.1.2-Linux).

Copy link
Collaborator

@lukeiwanski lukeiwanski Jan 22, 2017

Choose a reason for hiding this comment

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

After second look, that command looks correct.
Although, the instructions in os_setup.md are correct when it comes to Ubuntu 14.04 and 16.04.
What is your OS?

EDIT:
Ok I found what the issue is

if we do:
sudo mkdir /usr/local/computecpp
we need to change:
sudo cp -R ComputeCpp-CE-0.1.2-Linux /usr/local/computecpp to
sudo cp -R ComputeCpp-CE-0.1.2-Linux/* /usr/local/computecpp

or simply remove folder creation step.
Fixed in #33

Copy link
Contributor Author

@krikru krikru Jan 25, 2017

Choose a reason for hiding this comment

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

Running the cp command with no prior computecpp folder worked better for me. I also believe that adding a "*" to the source file path in the cp command as you suggested would have worked if the computecpp folder would have been there when running it.

lukeiwanski pushed a commit to lukeiwanski/tensorflow-opencl that referenced this pull request Jan 22, 2017
 - Fixes documentation issue mentioned in benoitsteiner#32 (comment)
This was referenced Jan 22, 2017
@benoitsteiner benoitsteiner force-pushed the master branch 2 times, most recently from b847171 to f694daa Compare February 14, 2017 23:35
@lukeiwanski
Copy link
Collaborator

@krikru ping on this issue.

@krikru
Copy link
Contributor Author

krikru commented Mar 7, 2017

The merge conflict in os_setup.md is resolved. Do you agree with the solution?

@lukeiwanski
Copy link
Collaborator

Could you remove ComputeCpp-CE-0.1.2-Linux ? As per previous comments?

@krikru
Copy link
Contributor Author

krikru commented Mar 8, 2017

Hm, it seems like GitHub keeps undoing my changes, or doesn't accept them, even though it indicates that the merge conflict in os_setup.md is successfully resolved when I press the "Resolve conflicts" button below, resolve the conflict and press "Mark as resolved." Do you know why that fails?

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.

3 participants