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

Add a test for training MNIST on CUDA and CPU that verifies same result as Jax #43

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sogartar
Copy link
Contributor

The two tests fail for different reasons. Things work OK except the procedure to initialize the optimizer.

@sogartar sogartar force-pushed the mnist-train-test branch 4 times, most recently from 83f9e31 to 77fbd83 Compare January 18, 2023 20:54
@sogartar sogartar force-pushed the mnist-train-test branch 2 times, most recently from 5dce0e9 to 2592d0a Compare January 28, 2023 00:18
@ThomasRaoux
Copy link

is there a way to get a test independent of JAX? If you don't have a simple way I'll try to make one later

…t as Jax

The two tests fail for different reasons. Things work OK except the
procedure to initialize the optimizer.
return next(batches)


def get_model():
Copy link
Contributor

@wangkuiyi wangkuiyi Feb 3, 2023

Choose a reason for hiding this comment

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

Thank you @sogartar for this pull request! It seems that this function duplicates with the one

def build_model():
init_random_params, predict = stax.serial(
Dense(1024),
Relu,
Dense(1024),
Relu,
Dense(10),
LogSoftmax,
)
Is it a good idea to add a test for that file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I copied the model from there. I guess mnist_export.py example can benefit from a test too.

@sogartar sogartar requested a review from rsuderman February 7, 2023 18:05
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.

4 participants