Skip to content
This repository was archived by the owner on Jul 1, 2023. It is now read-only.

Add liveness tracking to LazyTensor #190

Merged
merged 6 commits into from
Jun 10, 2019

Conversation

bgogul
Copy link
Contributor

@bgogul bgogul commented Jun 10, 2019

Tracks list of LazyTensorOperation instances held in LazyTensor instances. This is required to perform batch materialization of all live tensors during lazy evaluation.

@bgogul bgogul force-pushed the lazy_tensor_live branch from b0ff781 to ce2945d Compare June 10, 2019 18:38
@bgogul bgogul force-pushed the lazy_tensor_live branch from 5bd522f to 02456bc Compare June 10, 2019 18:52
@bgogul bgogul requested a review from rxwei June 10, 2019 18:52
@@ -16,6 +16,18 @@ import XCTest
@testable import TensorFlow
import CTensorFlow

struct LazyTensorOperationRef : Equatable, Hashable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, why not just extend LazyTensorOperation to Equatable and Hashable? The wrapper doesn't seem to buy you anything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I want that as well for some caching in lazy tensor implementation. However, not all properties of LazyTensorOperation will automatically conform to Equatable and Hashable. e.g., Attribute has some associated types that don't conform.

I will send out a separate PR with all the needed changes.

@bgogul
Copy link
Contributor Author

bgogul commented Jun 10, 2019

@rxwei Thanks for the review. I addressed your comments. PTAL.

@bgogul bgogul merged commit d538aab into tensorflow:master Jun 10, 2019
@bgogul bgogul deleted the lazy_tensor_live branch June 10, 2019 22:08
dan12411 pushed a commit to dan12411/swift-apis that referenced this pull request Jun 15, 2019
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.

2 participants