Addons: CLN: Streamline configure with python script

Created on 10 Jan 2020  路  10Comments  路  Source: tensorflow/addons

We make multiple calls to the python cli which imports TF multiple times and is slow and difficult to follow in the script. It would be better to call a python script which returns all the information we need in one call:
https://github.com/tensorflow/addons/blob/master/configure.sh#L107

build good first issue help wanted

All 10 comments

FLAGS=($(python -c 'import logging; logging.disable(logging.WARNING);import tensorflow as tf; print(" ".join(tf.sysconfig.get_compile_flags())," ".join(tf.sysconfig.get_link_flags()),tf.sysconfig.CXX11_ABI_FLAG)'))
TF_CFLAGS= ${FLAGS[0]}
TF_LFLAGS=${FLAGS[1]} 
TF_CXX11_ABI_FLAG=${FLAGS[2]}

I guess this snippet would work. But it has a very long line. Not sure if this version is desirable.

FLAGS=($(python -c 'import logging; logging.disable(logging.WARNING);import tensorflow as tf; print(" ".join(tf.sysconfig.get_compile_flags())," ".join(tf.sysconfig.get_link_flags()),tf.sysconfig.CXX11_ABI_FLAG)'))
TF_CFLAGS= ${FLAGS[0]}
TF_LFLAGS=${FLAGS[1]} 
TF_CXX11_ABI_FLAG=${FLAGS[2]}

I guess this snippet would work. But it has a very long line. Not sure if this version is desirable.

Yeah that would improve the performance, but as you mentioned it's unwieldy in the config script

Guys, a simple backslash does the trick. Isn't?

Guys, a simple backslash does the trick. Isn't?

I think a python script is more readable IMO. But any improvement would be good and taking a single python cli call formatted well in the shell script would be acceptable to me.

I think a python script is more readable IMO. But any improvement would be good and taking a single python cli call formatted well in the shell script would be acceptable to me.

Do you mean a complete transformation of that bash script into the python one? It's possible but will inevitable bring lots of new [subtle] bugs. :) I personaly strive to avoid such things unless there are strong reasons. Anyway, it's up to you.
As of a simple modification of the current script - I think @Gokkulnath should complete their job.

@Gokkulnath please submit a PR when time allows. Thanks!

Well. It's not a big hassle. Here it is. You can just download the script to check it out how it works. I was prone to make it as close to the original bash script as possible therefore it may look suboptimal and unusual from the python's best-practices standpoint.

@failure-to-thrive Please open a PR.

No problems. But I feel there is something more than just add a single file. What about original configure.sh? Should it be deleted? What if it is referenced from somewhere? What about documentation? If I should not worry about it, I will PR readily.

Don't worry about it, open the PR. I'd prefer if we continue the discussion there.

Was this page helpful?
0 / 5 - 0 ratings