Skip to content
This repository was archived by the owner on Mar 21, 2024. It is now read-only.

Conversation

@mfrancis95
Copy link
Contributor

No description provided.

@alliepiper
Copy link
Collaborator

LGTM -- builds and tests pass locally. Running CI under 28459137.

@alliepiper alliepiper added testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS). testing: gpuCI passed Passed gpuCI testing. to close: integrate Needs integration, then can be closed. labels May 27, 2020
@alliepiper alliepiper self-assigned this May 27, 2020
@alliepiper alliepiper removed the to close: integrate Needs integration, then can be closed. label May 28, 2020
@alliepiper
Copy link
Collaborator

@mfrancis95 Thanks for the patches! Could you merge #1169, #1171, and #1172 (and any similar additional updates) into one PR? It'll make it much easier on our internal testing/integration processes if these are all done in a single commit.

@jaredhoberock
Copy link
Contributor

I appreciate the effort in simplifying the codebase with these placeholders. The placeholder implementations are more complex than the simpler functors they replace. We should do some due diligence and check that these changes don't significantly increase compilation times.

Also, if it turns out that these changes make the old functors obsolete, they should be eliminated with these changes.

@alliepiper
Copy link
Collaborator

Good points, @jaredhoberock. I'll run my build-time benchmarks on these once they're are squashed down to a single PR/commit and report back.

Copy link
Collaborator

@alliepiper alliepiper left a comment

Choose a reason for hiding this comment

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

Please squash all open placeholder-related PRs into one PR with a single commit and remove the definitions of any functors that are no longer needed.

@alliepiper alliepiper removed testing: internal ci in progress Currently testing on internal NVIDIA CI (DVS). testing: gpuCI passed Passed gpuCI testing. labels May 28, 2020
@mfrancis95 mfrancis95 closed this Jun 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants