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

Executable tutorial #38

Merged
merged 9 commits into from
Aug 11, 2022
Merged

Executable tutorial #38

merged 9 commits into from
Aug 11, 2022

Conversation

paraynaud
Copy link
Member

  • environnement @example
  • add required dependencies

w :: Param{Matrix{Float32}} # parameters of the layers
b :: Param{Vector{Float32}} # bias of the layer
# w :: Param{CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}} # for GPU
# b :: Param{CuArray{Float32, 1, CUDA.Mem.DeviceBuffer}} # for GPU
Copy link
Member

Choose a reason for hiding this comment

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

How about parameterizing so that the user can use the struct on both CPU and GPU?

Copy link
Member Author

@paraynaud paraynaud Jul 7, 2022

Choose a reason for hiding this comment

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

We know that

Dense(i :: Int, o :: Int, f=sigm) = Dense(param(o, i), param0(o), f)

define the suitable Param structure.

You want to type it like

struct Dense{
    T <: Union{Matrix{Float32}, CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}}
    Y <: Union{Vector{Float32}, CuArray{Float32, 1, CUDA.Mem.DeviceBuffer}}
    }
  w :: Param{T}
  b :: Param{Y}

?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, for example.

Copy link
Member

Choose a reason for hiding this comment

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

Does

struct Dense{T, Y}
  w :: Param{T}
  b :: Param{Y}

work?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should work.
I typed it as accurate as I can.
I don't know about the performance (of Union), but I think it is more clear as it is than just T and Y.

docs/src/tutorial.md Outdated Show resolved Hide resolved
docs/src/tutorial.md Outdated Show resolved Hide resolved
@paraynaud
Copy link
Member Author

paraynaud commented Jul 15, 2022

I can't test it on GPU right now, all GPUs are used.

@paraynaud
Copy link
Member Author

I tested it on Atlas, and it worked.

README.md Outdated
set_size_minibatch!(DenseNetNLPModel, p) # p::Int > 1
```
## How to use
Check the [tutorial](https://juliasmoothoptimizers.github.io/KnetNLPModels.jl/dev/tutorial/).
Copy link
Member

Choose a reason for hiding this comment

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

Better point to stable.

@@ -1,5 +1,10 @@
[deps]
CUDA = "052768ef-5323-5732-b1bb-66c8b64840ba"
Copy link
Member

Choose a reason for hiding this comment

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

VMs don't always have a GPU. Why not run the docs on CPU?

Copy link
Member Author

@paraynaud paraynaud Jul 28, 2022

Choose a reason for hiding this comment

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

I added it to run the documentation (mainly the tutorial) on Atlas.
It will be remove by the next commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

(update) If we remove CUDA, we can't type the Dense structure like this:

struct Dense{
    T <: Union{Matrix{Float32}, CuArray{Float32, 2, CUDA.Mem.DeviceBuffer}}
    Y <: Union{Vector{Float32}, CuArray{Float32, 1, CUDA.Mem.DeviceBuffer}}
    }
  w :: Param{T}
  b :: Param{Y}

which makes documentation run fail.

Copy link
Member

Choose a reason for hiding this comment

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

Use the more general typing I suggested above. I don't think the Union adds anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is done, I think we are ready to go.

@@ -18,3 +22,6 @@ In addition, it provides tools to:
- switch the minibatch used to evaluate the neural network;
- change the minibatch size;
- measure the neural network's accuracy at the current `w`.

## How to use
Check the [tutorial](https://juliasmoothoptimizers.github.io/KnetNLPModels.jl/dev/tutorial/).
Copy link
Member

Choose a reason for hiding this comment

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

Why does this point to dev?

@@ -1,6 +1,7 @@
# KnetNLPModels.jl Tutorial

## Synopsis
KnetNLPModels is an interface between [Knet.jl](https://github.com/denizyuret/Knet.jl.git)'s classification neural networks and [NLPModels.jl](https://github.com/JuliaSmoothOptimizers/NLPModels.jl.git).
Copy link
Member

Choose a reason for hiding this comment

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

too much repetition

Copy link
Member

@dpo dpo left a comment

Choose a reason for hiding this comment

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

Great, thank you!

@dpo dpo merged commit eaad4c7 into main Aug 11, 2022
@dpo dpo deleted the executable_tutorial branch August 11, 2022 19:47
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.

None yet

2 participants