Synthetic multi view facial generator

[ekakalet]

This is a PR for synthetic multi-view facial image generator which will be a standalone tool of OPENDR generating data (facial images) for procedures such as e.g. training.

[passalis]

@ekakalet testing is now available again. So, when you are ready, you can use the "test sources" label to run the style checks and "test tools" to run the tool tests.
Please let us know when this PR is ready for review.

[ekakalet]

This PR is ready for review.

[stefaniapedrazzi]

Thank you.

I would add the license header to all the files developed in the OpenDR project also the ones in the projects folder.
This is the way you will set the copyright otherwise they won't have any owner.

[ekakalet]

Thank you.

I would add the license header to all the files developed in the OpenDR project also the ones in the projects folder.
This is the way you will set the copyright otherwise they won't have any owner.

The code in "opendr_internal/projects/data_generation/synthetic-multi-view-facial-image-generation/" is borrowed from another github repo (see README file). There are some doubts for the incorporation of license header. Only 3ddfa/SyntheticDataGeneration.py, testSyntheticDataGeneration.py, path_helper.py,path_helper2.py were produced in OPENDR

[stefaniapedrazzi]

The code in "opendr_internal/projects/data_generation/synthetic-multi-view-facial-image-generation/" is borrowed from another github repo (see README file). There are some doubts for the incorporation of license header.

What do you mean by "borrowed"?
Did you made any modification or just copied it as-is?
If you didn't apply any modifications, then you should add the other GitHub repo as submodule instead of copying all the files this repo.

In case you did some modifications, you have to also copy the LICENSE file from the original repository and better specify which modifications did you apply. Honestly from the README.md it is not very clear which files are copied and this is not good.
It would be much better to clearly specify which folder and files are copied and which one are new.

[ekakalet]

The code in "opendr_internal/projects/data_generation/synthetic-multi-view-facial-image-generation/" is borrowed from another github repo (see README file). There are some doubts for the incorporation of license header.

What do you mean by "borrowed"?
Did you made any modification or just copied it as-is?
If you didn't apply any modifications, then you should add the other GitHub repo as submodule instead of copying all the files this repo.

In case you did some modifications, you have to also copy the LICENSE file from the original repository and better specify which modifications did you apply. Honestly from the README.md it is not very clear which files are copied and this is not good.
It would be much better to clearly specify which folder and files are copied and which one are new.

Done. I have copied the license (in the README file) from the original repo that I used the code and I have mentioned its minor modifications and the original OPENDR functions. If there are issues, please let me know.

[stefaniapedrazzi]

Done. I have copied the license (in the README file) from the original repo that I used the code and I have mentioned its minor modifications and the original OPENDR functions. If there are issues, please let me know.

I'm not sure to understand which LICENSE file did you copy and where.
I only see this projects/data_generation/synthetic-multi-view-facial-image-generation/LICENSE file.
But it is quite confusing since it seems to apply to the whole files even if it is not the case.

I think that for each repository from where you copied some files, you should create a separate folder for the code and include the original LICENSE file taken from the source repository.
For example I would expect to see the LICENSE file taken from the 3DDFA repository in the folder "projects/data_generation/synthetic-multi-view-facial-image-generation/3ddfa/".

Additionally could you please also resolve the conflicts with the master branch?

[ekakalet]

Indeed, the file projects/data_generation/synthetic-multi-view-facial-image-generation/LICENSE applies to the whole files in the structure of the software folders as taken from the source repo. So I propose that it is safe to remain as is.
The coflict is resolved.

[stefaniapedrazzi]

Indeed, the file projects/data_generation/synthetic-multi-view-facial-image-generation/LICENSE applies to the whole files in the structure of the software folders as taken from the source repo. So I propose that it is safe to remain as is.

Well, to me it doesn't seem correct.
projects/data_generation/synthetic-multi-view-facial-image-generation/LICENSE is a Creative Commons License but only "Rotate-and-Render" and "SPADE" are released under this license.

The "SyncBN" and "3DDFA" code is released under MIT license.
MIT license states:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software."

So you have to copy the "SyncBN/3DDFA" MIT license alongside with the code you take from these repository, but for example in "3ddfa" folder I cannot find any reference to MIT license in this PR.
As is, it also seems that "SyncBN" and "3DDFA" are released under the Creative Commons License but this is wrong.

[ad-daniel]

Hi, is this PR ready for review? (conflict aside)

[ekakalet]

Hi, is this PR ready for review? (conflict aside)

Hi. yes this PR is ready for review.

[ad-daniel]

Hi, can I ask you to resync your branch with master? (and solve the conflicts)

Also I've noticed that there are many files that have been committed which I don't think are really needed (*.ply, *.mat, bash files, realign_lmk, and realign_lmk_ and a number of text files with intermediary results) and other files that I assume you used during the development but are no longer necessary: path_helper.py path_helper2.py. Some might be needed, but if so should be renamed properly (another example that needs to be renamed is: mobilenet_v1.py). In summary, could you make a general cleanup of the PR so as to maintain only what is relevant and to remove (or add to .gitignore) the rest? This page could help you clarify what is expected in terms of structure of your code.

I put the PR back on draft, whenever it's ready please let us know.

[ekakalet]

Hi, can I ask you to resync your branch with master? (and solve the conflicts)

Also I've noticed that there are many files that have been committed which I don't think are really needed (*.ply, *.mat, bash files, realign_lmk, and realign_lmk_ and a number of text files with intermediary results) and other files that I assume you used during the development but are no longer necessary: path_helper.py path_helper2.py. Some might be needed, but if so should be renamed properly (another example that needs to be renamed is: mobilenet_v1.py). In summary, could you make a general cleanup of the PR so as to maintain only what is relevant and to remove (or add to .gitignore) the rest? This page could help you clarify what is expected in terms of structure of your code.

I put the PR back on draft, whenever it's ready please let us know.

The PR was cleaned-up in the way that might be functional as a whole software structure with embedded respective paths and scripts' names in the folders of code. The scripts' renaming was done where it was permitted by the fact that they were used by internal parts of code. The use of code classes are described in the README file.

[ekakalet]

I think it's a good idea to create a new PR, since the toolkit was released already, this tool should now target the develop branch, not master (so when you create the branch, you have to create a branch from develop, not from master). You can keep this PR for reference, in case you need access to some of the files.

@ad-daniel With the latest modifications and bug fixes on the current version of this PR (without creating a new PR), it seems now to work

[ad-daniel]

Awesome! Thanks for your effort.
In theory we should create a new PR regardless so that this change targets the develop branch instead of master, but since it's a self-enclosed utility function it should be fairly safe to merge into master. (But hey, if you have the time to move it to develop I won't complain 😄 )
If you already did the clean up and the PR is ready for review I will look into it as soon as I can, let us know when it's ready

[ad-daniel]

Thank you! Just a few comments here and there:

I don't see a reason to push these folders into our repo, it just bloats the tookit:

• projects/data_generation/synthetic_multi_view_facial_image_generation/algorithm/Rotate_and_Render/misc/

Similarly, do we need to keep all files under here ?

• projects/data_generation/synthetic_multi_view_facial_image_generation/algorithm/DDFA/train.configs/

One of them is 25 MB, if it isn't used for our purpose, we shouldn't be including it.

EDIT: we should also add an entry in the changelog about this addition

old

[ekakalet]

Thank you! Just a few comments here and there:

I don't see a reason to push these folders into our repo, it just bloats the tookit:

• projects/data_generation/synthetic_multi_view_facial_image_generation/algorithm/Rotate_and_Render/misc/

Similarly, do we need to keep all files under here ?

• projects/data_generation/synthetic_multi_view_facial_image_generation/algorithm/DDFA/train.configs/

One of them is 25 MB, if it isn't used for our purpose, we shouldn't be including it.

EDIT: we should also add an entry in the changelog about this addition

The first folder is not necessary and the second we can add it as says the README file. @ad-daniel The changes are resolved.

[ad-daniel]

Thank you!

[ekakalet]

@passalis This PR is ready for the second review.

[passalis]

This is also fine with me. Thank you!