Make DALI buildable for Python 3.8 #1782

JanuszL · 2020-03-02T14:29:06Z

updates manylinux to build python 3.8.1 and remove not supported python versions
bumps up pybind11 version to 2.4.2
adjusts tests to python 3.8

Signed-off-by: Janusz Lisiecki jlisiecki@nvidia.com

Why we need this PR?

Pick one, remove the rest

It adds an ability to build DALI for python 3.8.x

What happened in this PR?

Fill relevant points, put NA otherwise. Replace anything inside []

What solution was applied:
updates manylinux to build python 3.8.1 and remove not supported python versions
bumps up pybind11 version to 2.4.2
Affected modules and functionalities:
manylinux, pybind11
Key points relevant for the review:
NA
Validation and testing:
CI build
Documentation (including examples):
NA

JIRA TASK: [DALI-1302]

dali-automaton · 2020-03-02T14:31:08Z

CI MESSAGE: [1163065]: BUILD STARTED

dali-automaton · 2020-03-02T15:37:11Z

CI MESSAGE: [1163186]: BUILD STARTED

dali-automaton · 2020-03-02T22:24:17Z

CI MESSAGE: [1163065]: BUILD FAILED

dali-automaton · 2020-03-03T01:17:06Z

CI MESSAGE: [1163186]: BUILD FAILED

lgtm-com · 2020-03-03T12:22:02Z

This pull request fixes 1 alert when merging b6512f468f1b97c6fe46d689f384f7a1a5769369 into 03373f9 - view on LGTM.com

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-03T12:54:24Z

CI MESSAGE: [1165490]: BUILD STARTED

dali-automaton · 2020-03-03T13:14:43Z

CI MESSAGE: [1165490]: BUILD FAILED

lgtm-com · 2020-03-03T13:45:05Z

This pull request fixes 1 alert when merging f869f74c32d2fcd20b26e4559a203b0923091d77 into 03373f9 - view on LGTM.com

fixed alerts:

1 for Unused import

lgtm-com · 2020-03-03T13:57:04Z

This pull request fixes 1 alert when merging 176868e15066a2731b69cbece16263c655062660 into 03373f9 - view on LGTM.com

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-03T13:57:12Z

CI MESSAGE: [1165588]: BUILD STARTED

dali-automaton · 2020-03-03T14:15:57Z

CI MESSAGE: [1165588]: BUILD FAILED

dali-automaton · 2020-03-03T15:04:54Z

CI MESSAGE: [1165708]: BUILD STARTED

lgtm-com · 2020-03-03T15:13:01Z

This pull request fixes 1 alert when merging 287c9be591cffc51137e2ff7b9b3286b6cfedc4d into 03373f9 - view on LGTM.com

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-03T16:29:30Z

CI MESSAGE: [1165708]: BUILD FAILED

dali-automaton · 2020-03-03T18:09:55Z

CI MESSAGE: [1166066]: BUILD STARTED

dali-automaton · 2020-03-03T19:19:15Z

CI MESSAGE: [1166066]: BUILD FAILED

dali-automaton · 2020-03-04T02:25:36Z

CI MESSAGE: [1167254]: BUILD STARTED

dali-automaton · 2020-03-04T03:35:15Z

CI MESSAGE: [1167254]: BUILD FAILED

dali-automaton · 2020-03-04T22:39:01Z

CI MESSAGE: [1169221]: BUILD STARTED

dali-automaton · 2020-03-05T02:06:38Z

CI MESSAGE: [1169221]: BUILD FAILED

lgtm-com · 2020-03-05T10:00:14Z

This pull request fixes 1 alert when merging 5b38aa99f7c49cbd302737c60d59538466c59207 into 4735a26 - view on LGTM.com

fixed alerts:

1 for Unused import

lgtm-com · 2020-03-05T10:15:10Z

This pull request fixes 1 alert when merging 2bbc59a3481fccbf3654944f01f8cf7869d9c325 into 4735a26 - view on LGTM.com

fixed alerts:

1 for Unused import

lgtm-com · 2020-03-10T17:45:46Z

This pull request introduces 1 alert and fixes 1 when merging c0f6709ea9b4f33ec8fc812c830e124ff328c9be into 01452f5 - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-10T18:35:13Z

CI MESSAGE: [1180895]: BUILD STARTED

dali-automaton · 2020-03-10T18:43:30Z

CI MESSAGE: [1180896]: BUILD STARTED

dali-automaton · 2020-03-10T19:18:29Z

CI MESSAGE: [1180897]: BUILD STARTED

dali-automaton · 2020-03-10T21:03:43Z

CI MESSAGE: [1180895]: BUILD PASSED

dali-automaton · 2020-03-10T21:28:55Z

CI MESSAGE: [1180896]: BUILD PASSED

dali-automaton · 2020-03-10T22:29:16Z

CI MESSAGE: [1180897]: BUILD PASSED

dali-automaton · 2020-03-11T00:16:58Z

CI MESSAGE: [1181805]: BUILD STARTED

lgtm-com · 2020-03-11T00:18:13Z

This pull request introduces 1 alert and fixes 1 when merging 5c690bcf9220ec2a8e7a9f05ce6f2842fcda965d into 01452f5 - view on LGTM.com

new alerts:

1 for Unused import

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-11T02:44:41Z

CI MESSAGE: [1181805]: BUILD PASSED

lgtm-com · 2020-03-11T22:11:59Z

This pull request fixes 1 alert when merging b2e1204b86cd6a795bec6955c129dd82ebba12c0 into 03b9368 - view on LGTM.com

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-11T22:18:05Z

CI MESSAGE: [1184134]: BUILD STARTED

dali-automaton · 2020-03-12T00:38:23Z

CI MESSAGE: [1184134]: BUILD PASSED

klecki · 2020-03-13T13:49:31Z

dali/test/python/test_video_pipeline.py

@@ -90,7 +90,7 @@ def check_videopipeline_supported_type(dtype):
    del pipe

 SUPPORTED_TYPES = [types.DALIDataType.FLOAT, types.DALIDataType.UINT8]
-ALL_TYPES = [v for k, v in types.DALIDataType.__dict__.items() if not re.match("__(.*)__", str(k))]
+ALL_TYPES = [v for k, v in types.DALIDataType.__dict__.items() if not (re.match("__(.*)", str(k)) or re.match("name", str(k)))]


Just use types.DALIDataType.__members__
https://pybind11.readthedocs.io/en/stable/classes.html#enumerations-and-internal-types

list(types.DALIDataType.__members__.values()) to be precise

klecki · 2020-03-13T15:12:36Z

qa/TL1_tensorflow-dali_test/test.sh

@@ -3,6 +3,9 @@ pip_packages=""
 target_dir=./docs/examples/use_cases/tensorflow/resnet-n

 do_once() {
+    if [ $($topdir/qa/setup_packages.py -n -u tensorflow-gpu --cuda ${CUDA_VERSION}) = -1 ]; then


Similar check is in the qa/test_template.sh

Can we maybe add an arg to setup_packages.py, that returns true/false depending whether we have anything to install or not?

In test_template.sh negative value is fine, for i in seq 0 $last_config_index; is just noop in this case. The only thing we do there is to make sure that last_config_index it not set to 0 when is negative and one_config_only was set.

lgtm-com · 2020-03-13T15:23:26Z

This pull request fixes 1 alert when merging 3b736010b43633a07ae5a2a89847a9a45edd617e into 452b359 - view on LGTM.com

fixed alerts:

1 for Unused import

klecki · 2020-03-13T15:34:52Z

qa/setup_packages.py

 def get_key_with_cuda(key, val_dict, cuda):
+    """Get get set of versions for highest matching cuda version available.


Suggested change

"""Get get set of versions for highest matching cuda version available.

"""Get the set of versions for highest matching cuda version available.

nitpicking

klecki · 2020-03-13T15:41:33Z

qa/setup_packages.py

@@ -177,8 +230,11 @@ def main():
    elif args.getall:
        print(get_all_strings(args.use, args.cuda))
    elif args.install >= 0:
-        if args.install > cal_num_of_configs(args.use, args.cuda):
+        if args.install > cal_num_of_configs(args.use, args.cuda) or args.install < 0:


We are in branch where args.install >= 0, it can't ever satisfy the or args.install < 0 clause.

klecki · 2020-03-13T15:46:45Z

qa/setup_packages.py

 def test_request(req, name, cuda):
+    """Check if given url to a package is avialable"""


Suggested change

def test_request(req, name, cuda):

"""Check if given url to a package is avialable"""

def test_request(py_ver, url, cuda):

"""Check if given `url` to a package of version `py_ver` is avialable"""

If you are already touching this, but I won't insist.

klecki

The setup_packages.py is getting a bit out of hand, but we agree that it's out of scope for this PR.

- updates manylinux to build python 3.8.1 and remove not supported python versions - bumps up pybind11 version to 2.4.2 - adjusts tests to python 3.8 Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

lgtm-com · 2020-03-13T22:39:06Z

This pull request fixes 1 alert when merging b1fef70 into 452b359 - view on LGTM.com

fixed alerts:

1 for Unused import

dali-automaton · 2020-03-13T23:04:32Z

CI MESSAGE: [1188857]: BUILD STARTED

dali-automaton · 2020-03-14T01:40:55Z

CI MESSAGE: [1188857]: BUILD PASSED

JanuszL force-pushed the update_python_builds branch from 1a7552a to b6512f4 Compare March 3, 2020 12:12

JanuszL force-pushed the update_python_builds branch from b6512f4 to f869f74 Compare March 3, 2020 13:37

JanuszL force-pushed the update_python_builds branch from f869f74 to 176868e Compare March 3, 2020 13:47

JanuszL force-pushed the update_python_builds branch from 176868e to 287c9be Compare March 3, 2020 15:03

JanuszL force-pushed the update_python_builds branch from 287c9be to 5b38aa9 Compare March 5, 2020 09:51

JanuszL force-pushed the update_python_builds branch from 5b38aa9 to 2bbc59a Compare March 5, 2020 10:05

JanuszL force-pushed the update_python_builds branch 2 times, most recently from 80c59b4 to c8bf59e Compare March 5, 2020 11:17

JanuszL force-pushed the update_python_builds branch from c0f6709 to 5c690bc Compare March 11, 2020 00:10

JanuszL force-pushed the update_python_builds branch from 5c690bc to b2e1204 Compare March 11, 2020 22:03

klecki reviewed Mar 13, 2020

View reviewed changes

JanuszL force-pushed the update_python_builds branch from b2e1204 to 3b73601 Compare March 13, 2020 15:13

klecki reviewed Mar 13, 2020

View reviewed changes

klecki approved these changes Mar 13, 2020

View reviewed changes

Make DALI buildable for Python 3.8

b1fef70

- updates manylinux to build python 3.8.1 and remove not supported python versions - bumps up pybind11 version to 2.4.2 - adjusts tests to python 3.8 Signed-off-by: Janusz Lisiecki <jlisiecki@nvidia.com>

JanuszL force-pushed the update_python_builds branch from 3b73601 to b1fef70 Compare March 13, 2020 22:31

JanuszL merged commit eef3c02 into NVIDIA:master Mar 14, 2020

JanuszL deleted the update_python_builds branch March 14, 2020 03:11

		def get_key_with_cuda(key, val_dict, cuda):
		"""Get get set of versions for highest matching cuda version available.

	"""Get get set of versions for highest matching cuda version available.
	"""Get the set of versions for highest matching cuda version available.

		def test_request(req, name, cuda):
		"""Check if given url to a package is avialable"""

Make DALI buildable for Python 3.8 #1782

Make DALI buildable for Python 3.8 #1782

Conversation

JanuszL commented Mar 2, 2020 • edited Loading

Why we need this PR?

What happened in this PR?

dali-automaton commented Mar 2, 2020

dali-automaton commented Mar 2, 2020

dali-automaton commented Mar 2, 2020

dali-automaton commented Mar 3, 2020

lgtm-com bot commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

lgtm-com bot commented Mar 3, 2020

lgtm-com bot commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

lgtm-com bot commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

dali-automaton commented Mar 3, 2020

dali-automaton commented Mar 4, 2020

dali-automaton commented Mar 4, 2020

dali-automaton commented Mar 4, 2020

dali-automaton commented Mar 5, 2020

lgtm-com bot commented Mar 5, 2020

lgtm-com bot commented Mar 5, 2020

lgtm-com bot commented Mar 10, 2020

dali-automaton commented Mar 10, 2020

dali-automaton commented Mar 10, 2020

dali-automaton commented Mar 10, 2020

dali-automaton commented Mar 10, 2020

dali-automaton commented Mar 10, 2020

dali-automaton commented Mar 10, 2020

dali-automaton commented Mar 11, 2020

lgtm-com bot commented Mar 11, 2020

dali-automaton commented Mar 11, 2020

lgtm-com bot commented Mar 11, 2020

dali-automaton commented Mar 11, 2020

dali-automaton commented Mar 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Mar 13, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klecki Mar 13, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

klecki left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Mar 13, 2020

dali-automaton commented Mar 13, 2020

dali-automaton commented Mar 14, 2020

JanuszL commented Mar 2, 2020 •

edited

Loading

klecki Mar 13, 2020 •

edited

Loading