Vertexwahn / rules_oidn

Bazel rules for Intel Open Image Denoise
https://vertexwahn.de/page/open_source/
Apache License 2.0
1 stars 1 forks source link

Provide Reference Test Reproducer #11

Closed ghost closed 1 year ago

ghost commented 1 year ago

Provide a denoising that functions, based on the original oidn cmake build.

This should be as reproducible cli instructions (ideally ubuntu and windows).

Vertexwahn commented 1 year ago

This is so to say the offical oidn denoiser: https://github.com/OpenImageDenoise/oidn/blob/master/apps/oidnDenoise.cpp Furthermore, there is https://declanrussell.com/portfolio/intel-open-image-denoiser-2/ - uses SConstruct but can server as a reference how things should work

ghost commented 1 year ago

(@Vertexwahn , this is still actual, despite #1 was fixed. You should not close issues that fast. Standard-procedure is to let the issue author close the issues, at least within collaborative repos)

This is so to say the offical oidn denoiser: https://github.com/OpenImageDenoise/oidn/blob/master/apps/oidnDenoise.cpp

this should do it.

as reproducible cli instructions

Could you provide the cli instructions here?

git clone https://github.com/OpenImageDenoise/oidn.git
cd oidn
<commang to get the image source>
<command to produce the denoised image>
<mention the location of the output image>
Vertexwahn commented 1 year ago

@abebeos Sorry for the fast closing. I considered the issue solved. Feel free to reopen it again.

I cannot provide CLI instructions. I did the OIDN build using the CMake GUI and manually Clicking on my Windows machine (and did beforehand manually install ISPC, oneTBB, etc.) - in the past I tried to come up with scripts for each step - e.g. building oneTBB -> https://github.com/Vertexwahn/Percdems/blob/master/Build_tbb-2018_U2_Visual%20Studio%2015%202017%20Win64.ps1 I can say one thing - it is not as easy as the Bazel approach:

cd ${HOME}
git clone https://github.com/Vertexwahn/rules_oidn.git
cd rules_oidn
cd tests
azel run --config=gcc11 //:example -- --input=${HOME}/rules_oidn/tests/data/cornell_box.naive_diffuse.box_filter.spp64.embree.exr --output=${HOME}/denoised_spp64.exr

That's exactly why I switch to Bazel and give up CMake -> https://medium.com/@Vertexwahn/bazel-as-an-alternative-to-cmake-fb7c86d95b48

ghost commented 1 year ago

Feel free to reopen it again.

It looks like github does not support this (one of its long lasting flaws, see https://github.com/isaacs/github/issues/583#issuecomment-910502478)

That's exactly why I switch to Bazel and give up CMake ->

I understand this, and if I had a personal project, I would most possibly prefer your rules_oidn build that links everything together (including ispc)

As for going on:

If you still have the goal to add (secondary) bazel support to repository with (primary) cmake support, then your build needs to be ideally 100% identical (possibly unreachable perfection: 100% identical binaries).

Starting point would be a reproducer/comparison for the primary build that's what this issue here is about. (Prerequisites can be installed manually, that's ok).

The relevant issue is #13.

All Good

If you're ok with your current (non identical, possibly different behavior under special circumstances) rules_oidn result, then everything is ok, and the reproducer here and issue #13 is not needed.

ghost commented 1 year ago

(@Vertexwahn , if #13 stays open, then it should be transferred to https://github.com/Vertexwahn/oidn - after enabling issues there)