NVlabs / curobo

CUDA Accelerated Robot Library
https://curobo.org
Other
796 stars 125 forks source link

Feature/support x86 64 #22

Closed blooop closed 1 year ago

blooop commented 1 year ago

Thanks for updating the docker files. After pulling the changes when running the build_dev_docker.sh I get:

~/curobo/docker$ ./build_dev_docker.sh 
./build_dev_docker.sh: line 23: [: ==: unary operator expected
./build_dev_docker.sh: line 28: [: ==: unary operator expected
Unknown Architecture

looking at the code I think its because my architecture is x86_64:

~/curobo/docker$ uname -m
x86_64

Also I need to chmod +x the build scripts to run them. Is there a reason they are not marked as executable?

balakumar-s commented 1 year ago

uname has been unreliable in some instances. I think we can make the build type explicit by using $1 be x86 for a x86 build. What do you think?

We could make it executable as well.

blooop commented 1 year ago

My fix makes it work for x86_64. I think that should cover most common architectures. It could be explicit as well but it would be nice to not need to specify the architecture by using the value from uname for the common case.

something like, use $1 if it is passed, otherwise use uname.

This pr marks all the scripts as executable

balakumar-s commented 1 year ago

I have added a check in the first part of the scripts to use uname if the argument is empty:


image_tag="x86"
isaac_sim_version=""
input_arg=$1

if [ -z "$input_arg" ]; then
    arch=`uname -m`

    echo "Argument empty, trying to build based on architecture"
    if [ $arch == "x86_64" ]; then
        input_arg="x86"
    elif [ $arch == "arm64" ]; then
        input_arg="aarch64"
    elif [ $arch == "aarch64" ]; then
        input_arg="aarch64"
    fi
fi

Are you done with all your changes? I can then apply this to the files.

blooop commented 1 year ago

Yes, I'm done now. I just fixed a couple other minor things after the docker image had finished building and I was able to run the script in its entirety.

balakumar-s commented 1 year ago

Thanks for the changes, I will apply my fixes and merge.