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

TF-LIFT For Windows #42

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

TF-LIFT For Windows #42

wants to merge 12 commits into from

Conversation

4xle
Copy link

@4xle 4xle commented Apr 13, 2020

Important Note: Prior to this PR, the development was done on a Windows box which unfortunately shorted during a power outage. The hard drive with these commits was originally thought to be unrecoverable, as was most of the system the code was developed on. I have no way to check the status or quality of the PR as it currently exists on a Windows box as the event prompted me to switch to Linux for development. The Anaconda environments should be re-creatable on Linux.

This collection of commits includes some QoL scripts as well as the results of digging through the code and adding comments or clarifying some logical operations with python functions instead of string addition. It also includes the yml and txt text files for recreating the Anaconda environments I was able to use to run the code with tensorflow-gpu 1.4.0 on Windows. The readme has been updated to reference these scripts for various training and testing commands.

I think I have stripped out any cruft or extraneous developments that aren't actually needed for the code, as this was originally a course project, but if I have missed anything please feel free to remove it. There are some minor changes to the eccv and helpers files under the datasets folder,

@4xle
Copy link
Author

4xle commented Apr 13, 2020

As mentioned in issue #28, I thought the code for the PR was lost.

@kmyi
Copy link
Contributor

kmyi commented Apr 13, 2020

Some of the changes here seem irrelevant. For example, there are permission changes. Can you remove those first?

@kmyi
Copy link
Contributor

kmyi commented Apr 13, 2020

Thank you for the PR by the way

@4xle
Copy link
Author

4xle commented Apr 13, 2020

Some of the changes here seem irrelevant. For example, there are permission changes. Can you remove those first?

Whoops, that might be from mounting the Windows backup onto my Linux system. I'll double-check. Don't usually have to think about permissions.

@4xle
Copy link
Author

4xle commented Apr 13, 2020

@kmyid Permissions appear to be uniform across my copy of the repository, can you call out anything specific for me to take a closer look at?

@kmyi
Copy link
Contributor

kmyi commented Apr 13, 2020

If you look at the file changes, for example
image

@4xle
Copy link
Author

4xle commented Apr 13, 2020

I've left the batch .py scripts as executable as that was how they were used, but non-executable files should now be permissioned correctly.

@kmyi
Copy link
Contributor

kmyi commented Apr 14, 2020

If you look at the diffs, there are still too many changes compared to the master branch, although it is hard to see what really changed. Can you also remove them? Only the parts where things have really changed should be highlighted in PRs so that we can easily keep track.

@4xle
Copy link
Author

4xle commented Apr 14, 2020 via email

@kmyi
Copy link
Contributor

kmyi commented Apr 14, 2020

Awesome thanks. No rush though. And thank you so much for the PR. It really helps.

Cheers,
Kwang

@4xle
Copy link
Author

4xle commented Apr 17, 2020

I'm going to test these commits before I call them functional, it will take me some time to rebuild and process everything though.

@kmyi
Copy link
Contributor

kmyi commented Apr 17, 2020

Thank you so much! ;)

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.

2 participants