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

Faster batch scaler tests #3604

Merged

Conversation

SkafteNicki
Copy link
Member

As discussed on slack the two slowest test has to do with the batch scaler.
This PR tries to bring their test time down.

@mergify mergify bot requested a review from a team September 22, 2020 08:59
@SkafteNicki SkafteNicki changed the title [WIP] faster batch scaler tests Faster batch scaler tests Sep 22, 2020
@SkafteNicki
Copy link
Member Author

@Borda I moved some of the batch scaling test from cpu to gpu (where they probably should have been all the time). It reduces the time of the two longest test by around 40 sec each, so total reduction by 1:30 min.

@williamFalcon williamFalcon merged commit 88e6b29 into Lightning-AI:master Sep 22, 2020
@Borda Borda added the ci Continuous Integration label Sep 22, 2020
@Borda
Copy link
Member

Borda commented Sep 22, 2020

@SkafteNicki can we also make the test smaller, just less data or larger steps?

@SkafteNicki
Copy link
Member Author

we could limit the size of the training set since after PR #3271 the search will stop when batch_size>=len(train_dataset), meaning lower test time

@awaelchli
Copy link
Contributor

@SkafteNicki I was thinking the same yesterday but then on the other hand, part of the test is also to check that the OOM handling goes correctly. so we would sacrifie that.

@SkafteNicki
Copy link
Member Author

@awaelchli also what I was thinking. Only other way to make the search stop earlier if we artificially filled up the gpu memory before running the search. Else let keep it as it is now (still reduced significantly compared to before).

@Borda Borda added this to the 0.9.x milestone Sep 22, 2020
@SkafteNicki SkafteNicki deleted the tuner/update_batch_scaler_test branch October 15, 2020 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants