Skip to content
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

Fix build and tensorflow 2.4 compatibility #95

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Timen
Copy link

@Timen Timen commented Feb 12, 2021

Small changes to fix build and tensorflow 2.4 compatibility. Configured using cmake 3.18.5, CUDA version = 11.2

@Grieverheart
Copy link

Hi @Timen. I also built dirt with the changes in this pull request, but running square_test.py failed. The rasteriser didn't seem to give the expected result. Did you try running it?

@Timen
Copy link
Author

Timen commented Mar 22, 2021

Hey @Grieverheart I mostly tested it with the deferred.py sample, so maybe try that one :)

@Grieverheart
Copy link

@Timen I had to make some changes to deferred.py, including adding 'tf.compat.v1.disable_eager_execution()' at the start of main to make it work, but the image I get does not like what you would expect:
deferred

@Timen
Copy link
Author

Timen commented Mar 22, 2021

@Grieverheart I updated the samples to tf 2.4.1 and fixed some issues saving images. However all 3 samples produced correct results for me as can be seen in these images:

test_deferred.py
test_textured.py

I can't say what the issue on your side is but I hope these results will encourage you to dig a bit deeper, as it should be possible to get it to work 👍

@Grieverheart
Copy link

@Timen Thanks for your help. After a lot of digging around, I found out that the problem I was having was that there seems to be an incompatibility of the default generated PTX with the GPU I'm using (a Tesla T4). Basically, I have to generate PTX for at least compute_70. In the end, I just set '-arch sm_75', and it works like a charm.

@pmh47
Copy link
Owner

pmh47 commented Mar 26, 2021

Thanks for the contribution @Timen -- I'll test it with some other versions of tensorflow before merging, to check everything is still ok there. In the changes to the samples, please don't add a dependency on PIL and numpy -- only some small changes to the tf calls for 2.x compatibility -- all the required functions are still present, just with slightly different names.

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.

None yet

3 participants