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

Tweak quickstart.md #2536

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

Conversation

mcabbott
Copy link
Member

@mcabbott mcabbott commented Dec 3, 2024

  • I think it's too obscure to say "cuDNN.jl has to be installed". We should just write the Pkg.add command out.
  • The line model(noisy |> gpu) |> cpu is doing too many things, simplify?
  • I don't like the "device = ..." pattern, but Carlo seems set on it, and added it here (instead of gpu). This at least tidies up a bit... most importantly never re-using x, y for things of different type.
  • Maybe the name of that should be a verb not a noun? Like move = get_device()? Not done yet.

@CarloLucibello
Copy link
Member

CarloLucibello commented Dec 4, 2024

Maybe the name of that should be a verb not a noun? Like move = get_device()? Not done yet

I would stick to device, it is very familiar to pytorch users, and x |> device is nice, x |> move is not, although move(x) could be better than device(x) . to_device = get_device() could be another option, bu I still preferer device, also because get_device() returns an XDevice object.

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

Successfully merging this pull request may close these issues.

2 participants