Skip to content

Add support for tfc.makeIteratorGetNextWithDatasets from graph operations #18223

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

Merged
merged 4 commits into from
Jul 26, 2018

Conversation

bgogul
Copy link
Contributor

@bgogul bgogul commented Jul 25, 2018

This is needed to enables strict deabstraction in dataset.swift test.

@bgogul bgogul changed the title Add support for tfc.makeIteratorGetNextWithDatasets from graph operations. [NOT_READY] Add support for tfc.makeIteratorGetNextWithDatasets from graph operations. Jul 25, 2018
@bgogul bgogul force-pushed the unknown_array_attribute_8331 branch from 88a6ecb to a2d859e Compare July 25, 2018 21:01
@bgogul
Copy link
Contributor Author

bgogul commented Jul 25, 2018

@swift-ci please test tensorflow

@bgogul bgogul changed the title [NOT_READY] Add support for tfc.makeIteratorGetNextWithDatasets from graph operations. Add support for tfc.makeIteratorGetNextWithDatasets from graph operations Jul 25, 2018
@bgogul bgogul requested a review from mhong July 25, 2018 21:24
@@ -584,7 +584,11 @@ struct TFGraphLowering : public SILInstructionVisitor<TFGraphLowering, GLStatus>
///
/// FIXME: Dissolve this builtin into a set of finer-grained, composable
/// features.
GLStatus visitTFDataset(BuiltinInst *inst);
template <typename Inst>
Copy link

Choose a reason for hiding this comment

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

if it can simplify the patch (e.g. remove templating), pls feel free to remove the support for builtin inst, and only support graph_op.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me leave the code as is for now. We can clean it up when strict deabstraction is enabled by default.

// Even when this built-in returns multiple tensors, they are always presented
// by a single tuple.
std::vector<TF_DataType> outputTypes;
for (const SILValue& result : inst->getResults()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: &result

@bgogul bgogul merged commit 182245a into swiftlang:tensorflow Jul 26, 2018
@bgogul bgogul deleted the unknown_array_attribute_8331 branch July 27, 2018 06:04
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.

3 participants