Skip to content

Feature Request: GPUOptions for Go binding #22926

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

Open
mattn opened this issue Oct 12, 2018 · 109 comments
Open

Feature Request: GPUOptions for Go binding #22926

mattn opened this issue Oct 12, 2018 · 109 comments
Labels
good first issue Good first issue for newcomers. stat:contribution welcome Status - Contributions welcome type:feature Feature requests

Comments

@mattn
Copy link
Contributor

mattn commented Oct 12, 2018

Current implementation of Go binding can not specify options.

GPUOptions struct is in internal package. And go generate doesn't work for protobuf directory. So we can't specify GPUOptions for NewSession.

@tensorflowbutler tensorflowbutler added the stat:awaiting response Status - Awaiting response from author label Oct 12, 2018
@tensorflowbutler
Copy link
Member

Thank you for your post. We noticed you have not filled out the following field in the issue template. Could you update them if they are relevant in your case, or leave them as N/A? Thanks.
Have I written custom code
OS Platform and Distribution
TensorFlow installed from
TensorFlow version
Bazel version
CUDA/cuDNN version
GPU model and memory
Exact command to reproduce
Mobile device

@ymodak ymodak assigned jhseu and unassigned ymodak Oct 17, 2018
@ymodak ymodak added the stat:awaiting tensorflower Status - Awaiting response from tensorflower label Oct 17, 2018
@jhseu jhseu added stat:contribution welcome Status - Contributions welcome and removed stat:awaiting response Status - Awaiting response from author stat:awaiting tensorflower Status - Awaiting response from tensorflower labels Oct 18, 2018
@jhseu jhseu removed their assignment Oct 18, 2018
@frreiss
Copy link
Contributor

frreiss commented Dec 3, 2018

This problem appears to be broader than just specifying GPUOptions for TensorFlow sessions. There is no native Go API for passing any options when creating a session. The user must create the binary representation of a ConfigProto protocol buffer outside of the TensorFlow Go API.

See, for example, the test case TestSessionConfig in session_test.go:

func TestSessionConfig(t *testing.T) {
	// Exercise SessionOptions.
	// Arguably, a better API would be for SessionOptions.Config to be the
	// type generated by the protocol buffer compiler. But for now, the
	// tensorflow package continues to be independent of protocol buffers
	// and this test exercises the option since the implementation has a
	// nuanced conversion to C types.
	//
	// Till then, the []byte form of Config here was generated using a toy
	// tensorflow Python program:
	/*
	 import tensorflow
	 c = tensorflow.ConfigProto()
	 c.intra_op_parallelism_threads = 1
	 print c.SerializeToString()
	*/
	graph := NewGraph()
	c, err := Const(graph, "Const", int32(14))
	if err != nil {
		t.Fatal(err)
	}
	opts := SessionOptions{Config: []byte("(\x01")}
[...]

Would the TensorFlow maintainers accept a non-Google contribution that added the ability to specify session options using pure Go code? This functionality would require generating instances of the ConfigProto protocol buffer defined in config.proto. I can see two ways to generate these protocol buffers: Either add a build target to generate Go bindings for the files in tensorflow/core/protobuf; or add Go wrappers for the generated C++ code in tensorflow/core/protobuf/config.pb.h

@ymodak ymodak added the type:feature Feature requests label Dec 3, 2018
@ymodak
Copy link
Contributor

ymodak commented Dec 3, 2018

@asimshankar Can you please take a look? Thanks!

@alextp alextp added the good first issue Good first issue for newcomers. label Mar 4, 2019
@alextp
Copy link
Contributor

alextp commented Mar 4, 2019

@frreiss we'll definitely accept a Go-only API to configure tensorflow.

@frreiss
Copy link
Contributor

frreiss commented Mar 13, 2019

I am looking into this.

@frreiss
Copy link
Contributor

frreiss commented Mar 13, 2019

Created PR #26682 with a Go API to create ConfigOptions protocol buffer messages. I made the changes as narrow in scope as I could.

@mattn
Copy link
Contributor Author

mattn commented Mar 30, 2019

Closed by #26682

@frreiss Thank you

@mattn mattn closed this as completed Mar 30, 2019
@mattn
Copy link
Contributor Author

mattn commented Apr 5, 2019

#26682 is reverted 6e9cb40#diff-dcd28ad951bd17e9c512d3b564640bab

@mattn mattn reopened this Apr 5, 2019
@imrahul361

This comment has been minimized.

@mattn
Copy link
Contributor Author

mattn commented Sep 30, 2020

Yes please.

@imrahul361
Copy link

May I know how to reproduce the issue?

@mattn
Copy link
Contributor Author

mattn commented Oct 1, 2020

Sorry, I don't know why the changes was reverted.

#26682 (comment)

@imrahul361

This comment has been minimized.

@AdeshKhandait

This comment has been minimized.

@Aditya-Komaravolu

This comment has been minimized.

@himanshu007-creator

This comment has been minimized.

@rajveer43

This comment has been minimized.

@JAYANTHNITW

This comment has been minimized.

@Antonyj12

This comment has been minimized.

@groovitz

This comment has been minimized.

@devanshi-code18

This comment has been minimized.

@rayyan21d

This comment has been minimized.

@party-dev-gamer-karannaidu

This comment has been minimized.

@keshavbnsl102

This comment has been minimized.

@Bhalchandra-patil03

This comment has been minimized.

@diaconumadalina

This comment has been minimized.

@manojgowda5

This comment has been minimized.

@AnmolArora15

This comment has been minimized.

@mohmmadAyesh

This comment has been minimized.

@shah9678

This comment has been minimized.

@willyg23

This comment has been minimized.

@yashpratap914

This comment has been minimized.

@joydeep049

This comment has been minimized.

@harshkumar-2005

This comment has been minimized.

@koliasa
Copy link

koliasa commented Nov 12, 2024

Thanks for the clarification! This does seem like a limitation of the current Go binding implementation. Since the GPUOptions struct is in the internal package, we can't directly pass it when creating a session with NewSession.

Regarding go generate and protobuf: this is also an important issue, as we need access to the appropriate configurations in protobuf to properly specify GPU options.

One potential solution might be to make GPUOptions accessible through a public API or consider adding custom options to NewSession via another configuration mechanism (e.g., a configuration file or a specialized getter method).

@SurinderSinghSaby

This comment has been minimized.

@Flutevishnu

This comment has been minimized.

@SurinderSinghSaby

This comment has been minimized.

@siya100

This comment has been minimized.

@sananthanarayan

This comment has been minimized.

@arzoo0511

This comment has been minimized.

@Filip-Nachov

This comment has been minimized.

@AshayV04

This comment has been minimized.

@aayushkeshari

This comment has been minimized.

@mihaimaruseac
Copy link
Collaborator

Locking this due to so many spammers. You should really be proud of yourselves.

As a reminder: in open source, you don't ask to be assigned some work. You deliver said work.

@tensorflow tensorflow locked as spam and limited conversation to collaborators May 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
good first issue Good first issue for newcomers. stat:contribution welcome Status - Contributions welcome type:feature Feature requests
Projects
None yet