tom94/practical-path-guiding

Code of "Practical Path Guiding for Efficient Light-Transport Simulation"

practical-path-guiding's Issues

Question regarding statisticWeights in building quadtree

Hello Tom,

Thank you for your great work.
I hold a question regarding the your quadtree resetting part. In your quadtree reset function, you will refine both the building quadtree and sampling quadtree and then reset the building quadtree to zero if I understand it correctly. I wonder why you didn't reset the statisticWeights to zero in building quadtree as well (while I don't think it affect performance a lot)?

Thanks for your help:)

Cheers

Adjoint Russian Roulette

Hello!
First of all: thank you for the great paper and for sharing this implementation.

However, your implementation of Adjoint Russian Roulette seems to contain a bug:

practical-path-guiding/mitsuba/src/integrators/path/guided_path.cpp

Lines 1878 to 1880 in 6058451

	if (tMax < wMin) {
	successProb = wMin / tMax;
	}

From my understanding, this should read successProb = tMax / wMin.
In its current form, successProb is always greater than 1, which essentially disables RR.

Question about the KL Divergence

Hi Tom,

Thanks for your great work.
Regarding your gradient descent for KL Divergence, I wonder is there any possibility that your optimized bsdfSamplingFraction will fall into a local optimization because the whole sphere domain share a same bsdfSamplingFraction?

Details on radiance caching

Hi Thomas,

I looked at the code and there is one part that I don't quite understand:
In DTreeWrapper's record function:
Float irradiance = rec.radiance / rec.woPdf;

Based on my understanding you recorded this "irradiance" into the dTree. Why not directly record rec.radiance? I tried but looks like if I record the rec.radiance, then the result will have a lot more noise. And I'm a bit confused here. Based on the definitions, shouldn't the incoming irradiance and radiance be related by a cosine term? Why are they related by this woPdf?

Thanks!!!

About Vorba's guided path tracer

Hi Tom,

Thanks for sharing the great work! I found that in your scene XML files, you have the commented out parameters for Vorba's guided path tracer. Did you implement one on your own or use their releasing code? Since I use Linux, Vorba's original code cannot be directly compiled on my computer and looks like many things need to be modified to make it work on Linux, for example, their LibImportance library (maybe I'm wrong). Do you have Vorba's work integrated into your framework that could be run on Linux? Thanks!

Best,
Shilin

About the division of pdf in the record code

Dear Tom,

Hello, Thank you for sharing this great work.
I have one confusion about your code:

About the "record" function in the "DTreeWrapper", I wonder why you decided to divide the woPdf here instead of using the radiance?

Float irradiance = rec.radiance / rec.woPdf;

building.recordIrradiance(dirToCanonical(rec.d), irradiance, rec.statisticalWeight, directionalFilter);

reason about not using NEE in Path Guiding

Dear Tom,

Hello. After checking your source code, I found that you didn't do NEE during the rendering, could you tell me the reason about not doing NEE?

Thanks.

When the distribution of radiance is extremely narrow, the pdf value will be extremely large.

Hi, Tom:
Thanks for your great work and sharing the codes. When I use path guiding in my own project, I find some problems as described above.

When the distribution of radiance is extremely narrow (B, C, D and A2, A3, A4 are all zero , A1 is not zero ), so the pdf is 16 / (4 * pi). When the dtree is deeper, the pdf value will be larger, and Radiance / pdf will nearly zero too.
Do I miss something? Any suggestion would be appreciated.

Binary tree vs Octree

Hi Thomas,

I'm wondering is there any specific reasons that you choose binary tree over Octree as the spatial representation? I'm not sure but maybe the spatial adaptation can be learned faster if using Octree? I saw that in the paper you mentioned that "Effectively, our threshold ensures that the total number of leaf nodes and the amount of samples in each leaf both grow at the same rate across iterations." I don't know if similar statement can be applied to Octree instead of binary tree. I would like to try Octree but I'm not sure how to set up the SPP and threshold correctly so that this property still hold (maybe change base 2 to base 8?). Please correct me if I'm wrong. If you had tried that, maybe you could share some experience with me?

Thanks!!!

Best,
Shilin

Reason about setting new vertex with zero radiance

Hi Tom,

Thanks for your great work.
In your code of evaluating direct light using BSDF sampling with MIS, you add a new vertex after recording radiance. I wonder why you set the new vertex with zero radiance when always doing NEE.

Thanks.

Spectrum L = throughput * value * weight;
if (!L.isZero()) {
    recordRadiance(L);
}

if ((!isDelta || m_bsdfSamplingFractionLoss != EBsdfSamplingFractionLoss::ENone) && dTree && nVertices < MAX_NUM_VERTICES && !m_isFinalIter) {
    if (1 / woPdf > 0) {
        vertices[nVertices] = Vertex{
            dTree,
            dTreeVoxelSize,
            ray,
            throughput,
            bsdfWeight * woPdf,
            (m_nee == EAlways) ? Spectrum{0.0f} : L,
            woPdf,
            bsdfPdf,
            dTreePdf,
            isDelta,
        };

        ++nVertices;
    }
}

Scons build errors

Dear Tom:

Hi!

I cloned the code & dependencies from you, but when I build via Scons, some errors occured:

src\libpython\core.cpp(2244) : error C2780: 'boost::python::class_<mitsuba::AABB1,boost::python::detail::not_specified,boost::python::detail::not_specified,boost::python::detail::not_specified> &boost::python::class_<mitsuba::AABB1,boost::python::detail::not_specified,boost::python::detail::not_specified,boost::python::detail::not_specified>::def(const char *,Fn,const A1 &,const A2 &,const A3 &)' : expects 5 arguments - 2 provided dependencies\include\boost/python/class.hpp(266) : see declaration of 'boost::python::class_<mitsuba::AABB1,boost::python::detail::not_specified,boost::python::detail::not_specified,boost::python::detail::not_specified>::def'

But I can build the original Mitsuba 0.5.0 successfully.
Any clues?

Windows10
VS2013
Python 2.7.15

Big thanks.

Reason for dropping the previous tree

Hi Thomas,

Sorry to open another issue...

I can see you keep two SD-Trees, one for guiding and one for irradiance deposit. If I understand the code correctly, after each iteration you drop the previous tree and build a new tree using the new samples from this new iteration. I guess this is because after you subdivide the tree, leaf nodes are smaller so previously learned sampling maps are not accurate anymore. But do you really need to drop the entire tree? I think the previously learned sampling map still contains a lot of useful information & valid samples and if you drop them completely, then you need to trace them again in the next iteration, which may introduce some redundancy here since some paths may be traced repeatedly. Is there any way to gradually evolve the sampling map so they can adapt to smaller leaves faster without starting from scratch (empty tree)?

Thanks!

Questions about Sample Deposit

Hi Thomas,

Thank you for sharing the code! I'm a beginner in path guiding, so I'm a bit confused about some parts of the code:

1. When depositing samples into the quadtree, I noticed that you computed the Monte Carlo estimated irradiance as Float irradiance = rec.radiance / rec.woPdf. And I can see woPdf is defined as woPdf = bsdfSamplingFraction * bsdfPdf + (1 - bsdfSamplingFraction) * dTreePdf. So if I understand Monte Carlo theory correctly, this irradiance should be the estimated sum of all the incoming directions (full sphere) right? And then it is recorded to the SDTree by building.recordIrradiance(). Currently I'm using the default ENearest mode, so I believe I ended up with calling the function m_nodes[0].record(p, irradiance * statisticalWeight, m_nodes), which is defined as:

void record(Point2& p, Float irradiance, std::vector<QuadTreeNode>& nodes) {
        SAssert(p.x >= 0 && p.x <= 1 && p.y >= 0 && p.y <= 1);
        int index = childIndex(p);

        if (isLeaf(index)) {
            addToAtomicFloat(m_sum[index], irradiance);
        } else {
            nodes[child(index)].record(p, irradiance, nodes);
        }
    }

So why is the estimated irradiance of all incoming directions deposited into a leaf node in the quadtree? From my understanding, each quad in the quadtree needs to store the irradiance of the solid angle corresponding to that quad. For example, a bigger quad should have a larger irradiance than a smaller quad.

2. I also found a statisticalWeight variable in the code, and I don't know what it represents. I printed its value and looks like it is either 1.0 or 0.5.

I definitely missed some important details in your code since I'm new to this field. It would be great if you can clarify these. Thanks a lot!

Thanks,
Emily