Skip to content

Re-enable some of the recently disabled cuda tests#7873

Merged
RyanUnderhill merged 11 commits intomasterfrom
ryanunderhill/cuda_tests
Jun 3, 2021
Merged

Re-enable some of the recently disabled cuda tests#7873
RyanUnderhill merged 11 commits intomasterfrom
ryanunderhill/cuda_tests

Conversation

@RyanUnderhill
Copy link
Contributor

Description: As promised, this re-enables some of the disabled cuda tests due to the shared cuda provider changes

There are still 3 files to go after this change

@RyanUnderhill RyanUnderhill requested a review from a team as a code owner May 28, 2021 06:44
@RyanUnderhill RyanUnderhill reopened this Jun 2, 2021
CUDAExecutionProviderInfo epi;
epi.device_id = 0;
EXPECT_TRUE(session_object.RegisterExecutionProvider(std::make_unique<CUDAExecutionProvider>(epi)).IsOK());
EXPECT_TRUE(session_object.RegisterExecutionProvider(DefaultCudaExecutionProvider()).IsOK());
Copy link
Contributor

Choose a reason for hiding this comment

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

It is good. But if you help us change it to

  ASSERT_STATUS_OK(session_object.RegisterExecutionProvider(DefaultCudaExecutionProvider());

It will be even better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is even better now

snnn
snnn previously approved these changes Jun 2, 2021
snnn
snnn previously approved these changes Jun 2, 2021
snnn
snnn previously approved these changes Jun 3, 2021
@RyanUnderhill
Copy link
Contributor Author

I put a way to call cleanup functions inside of test_main.cc, but not sure if there's a good place to put the prototype for 'CallOnTestExit'. Currently the prototype is just declared where it is used.


// We keep a list of functions to call on exit before we destroy the OrtEnv.
// This is needed to do any cleanup that must be done before the OrtEnv gets destroyed and all shared providers get unloaded.
static std::vector<std::function<void()>> exit_functions;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make our unit test program too complicated?

Copy link
Contributor

Choose a reason for hiding this comment

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

If unit tests need special handlings like this, probably end users need such things too. It doesn't look good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so, but since we love using global variables, we need a way to clean things up before we destroy the OrtEnv.

@RyanUnderhill RyanUnderhill merged commit 8f8b930 into master Jun 3, 2021
@RyanUnderhill RyanUnderhill deleted the ryanunderhill/cuda_tests branch June 3, 2021 21:28
@RyanUnderhill
Copy link
Contributor Author

The single test failure is a known one occurring without my changes.

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.

2 participants