Pipeline: Simplify wrapper

nielsboecker / HoloRepository-Core

A system for transforming medical imaging studies into holograms, storing them in the cloud and providing them to other systems.

GNU Affero General Public License v3.0

6 stars 8 forks source link

Pipeline: Simplify wrapper #82

Closed ansonwong9695 closed 5 years ago

ansonwong9695 commented 5 years ago

add Simplify binary to third_party
add its wrapper in wrappers
add SIMPLIFICATION_RATIO in env as default value
add simplify calls to all pipelines

nielsboecker commented 5 years ago

Btw @ansonwong9695, @UdomkarnBoonyaprasert I merged this locally with the tests branch and the 3 pipeline tests fail. I think it's because the path to the temp.obj file is broken due to the way you use cwd (=> it will look for the temp file as subdirectory of the cwd path, which is of course wrong). So I suspect that once you follow my PR suggestions, it will also fix that bug.

nielsboecker commented 5 years ago

TODO: Once my comments are sorted out, change PR target to dev

UdomkarnBoonyaprasert commented 5 years ago

I added a sample obj file (tests/utils/teddy.obj) for testing. It's around 90 KB, would this be ok?

nielsboecker commented 5 years ago

Yes that sounds like a good idea. But currently, the test fails, maybe due to absolute vs relative path to test data.

boonwj commented 5 years ago

The different workflows do appear to work in docker. There's just a new configuration field (SIMPLIFICATION_RATIO) that I need to take care of when I deploy it to kubes.

Also, there does not seem to be any documentation on the individual configuration fields. Pipeline's readme and the configuration section only mentioned that .env is the configuration file.

If there's time, it will be nice to document the individual fields.