Remove unwrap from examples

[bheisler]

We shouldn't be encouraging users to unwrap errors, but to use ? instead. Go through all of the examples in the Rustdoc comments and change them to use ? instead of unwrap(), like so:

# use std::error::Error;
# pub fn main() -> Result<(), Box<Error> {
// Example goes here
extern crate rustacuda;
let _ctx = rustacuda::quick_init()?;
# }

[alphastrata]

Is this still open?
There don't seem to be any .unwrap()s in the launch.rs from the examples folder...
I'd like to take a crack at this (but I'm probably a little below the level of a beginner... )

[bheisler]

Hey, thanks for your interest!

Yes, this is still open. I meant that we should remove unwrap from the examples in the rustdoc comments. I'll update the description to clarify.

[alphastrata]

I'm happy to give it a go! hopefully, the 'beginner' tag isn't too far out of my reach!

[alphastrata]

Can I take a stab at this?

[bheisler]

Please do!

[alphastrata]

I finally got this compiling this evening!!! first contribution to open source here we come.

[bheisler]

If possible, could you post all of the problems you ran into while trying to get it to compile? It might help us make it easier for others.

[alphastrata]

I think they were all CUDA install related:

• When installing Cuda I customized the install - unchecking driver & visual studio options as I already had 2015, 2013 and 2017 on my machine.

• I had to set the CUDA_path a few times, was unable to do so via command line (unsure why...) so used the environment settings GUI from the windows start menu

• I use vs code as my editor and its integrated terminal most of the time so I had to change the path from the default cmd to the visual studio 2015 developer tools cmd line in order to get .cu files compiling properly.

• At this stage I have three CUDA toolkits installed (all default settings throughout the installation. I am unsure which of the (8.1, 9.0 and 10.0) that RustaCuda is now calling. doing a nvcc --version tells me that CUDA 10.0 is the active version.

• If someone is doing a vanilla install of the CUDA toolkit they'll probably not run into the issues I had, if they're familiar with this sort of toolchain setup they'll definitely be fine.

-I'm on windows 10 developer preview. [ Ryzen 7 1800x, 32GB RAM, GTX1080ti ]

[bheisler]

Hmm. My understanding is that the CUDA driver and graphics driver are not the same thing, so you might need the CUDA driver as well. I could be wrong though.

Anyway, yeah, it sounds like more of a problem with NVIDIA's documentation than ours. The one thing we can do is automatically detect common installation paths so that the user doesn't have to set CUDA_LIBRARY_PATH, and I think some folks are already working on that for cuda-sys.

[alphastrata]

@bheisler this kinda thing yeah?

fn main() -> Result<(), Box<dyn Error>> {
    rustacuda::init(rustacuda::CudaFlags::empty())?;
    let device = Device::get_device(0)?;
    let context =
        Context::create_and_push(ContextFlags::MAP_HOST | ContextFlags::SCHED_AUTO, device)?;
    // call RustaCUDA functions which use the context
    // The context will be destroyed when dropped or it falls out of scope.

    Ok(drop(context))
}

So, for the docs using # to hide the main(s)

//! # use std::error::Error;
//! # use rustacuda;
//! # use rustacuda::context::{Context, ContextFlags};
//! # use rustacuda::device::Device;
//! # fn main() -> Result<(), Box<dyn Error>> {
//!     rustacuda::init(rustacuda::CudaFlags::empty())?;
//!     let device = Device::get_device(0)?;
//!     let context =
//!         Context::create_and_push(ContextFlags::MAP_HOST | ContextFlags::SCHED_AUTO, device)?;
//!     // call RustaCUDA functions which use the context
//!     // The context will be destroyed when dropped or it falls out of scope.
//! 
//! #    Ok(drop(context))
//! #}

It looks like some of the content may be more difficult than propogating dyn errors,
is our goal to remove ALL .unwrap()s? for example:

// Create and pop contexts for each device
    let contexts = Device::devices()?
        .map(|device| {
            let ctx = Context::create_and_push(
                ContextFlags::MAP_HOST | ContextFlags::SCHED_AUTO,
                device.unwrap(), // One
            ).unwrap(); // Two
            ContextStack::pop().unwrap(); // For One, Two & Three, are .expects best here or with (mroe) research will I find specific cuda erros from the errors.rs
            ctx
        })
        .collect::<Vec<Context>>();
    CurrentContext::set_current(&contexts[0])?;
    Ok(())

Hope I'm on the right track & that this isn't causing more work than just removing them yourself :P

[bheisler]

That looks about right, yeah.

I would like to remove all of the unwrap()s in the examples, but don't worry about it too much if you have to leave some of them for now. I can always go back and fix up the tricky ones myself later.

For the one you posted, I would change it to something like this:

// Create and pop contexts for each device
let mut contexts = vec![];
for device in Device::devices()? {
    let device = device?;
    let ctx = Context::create_and_push(
        ContextFlags::MAP_HOST | ContextFlags::SCHED_AUTO,
        device)?;
    ContextStack::pop()?;
    contexts.push(ctx);
}
CurrentContext::set_current(&contexts[0])?;

EDIT: For most of these examples, the Ok(()) at the end should be hidden as well. I see you've changed it to Ok(drop(context)) in that one example - that should be:

drop(context);
# Ok(())

[alphastrata]

Should I remove the unwraps from the #[test]s as well? for example from this (src/module.rs):

#[test]
    fn test_copy_from_module() {
        let _context = quick_init();

        let ptx = CString::new(include_str!("../resources/add.ptx")).unwrap();
        let module = Module::load_from_string(&ptx).unwrap();

        let constant_name = CString::new("my_constant").unwrap();
        let symbol = module.get_global::<u32>(&constant_name).unwrap();

        let mut constant_copy = 0u32;
        symbol.copy_to(&mut constant_copy).unwrap();
        assert_eq!(314, constant_copy);
    }```

[bheisler]

Huh. I had no idea that returning Results from tests was allowed now. Neat!

In that case, sure. Might as well update the tests too.