-
Notifications
You must be signed in to change notification settings - Fork 296
Fix weights in sample_wavelengths
when using SRF.
#1710
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
base: master
Are you sure you want to change the base?
Conversation
Hi @elerac, It seems there might be a few misunderstandings. I'll try to clarify them for you:
Based on your test, if you change the sensor's SRF to something other than the RGB sensitivities, it makes sense that the final RGB image will be different. This is because you are changing the sensitivity. The To summarize, SRFs define a sensor's sensitivity, and we use this information to create a tailored importance sampling scheme. I believe the misunderstanding stems from thinking that SRFs only model the sampling weights. I hope this helps clarify things. |
Thank you, @mcrescas, for the clarification! If I understand correctly, I know the behavior of importance sampling and the weighting scheme in specfilm. My modification does not change the final rendered images in specfilm. To explain the attached images, I specify one SRF (not multiple) in the sensor plugin with hdrfilm (not specfilm). As you mentioned, the importance sampling scheme is changed based on the provided SRF distribution. However, there is a mismatch of the corresponding weights; the current weight is an integral value, which is a constant that does not depend on wavelength. For sure, inside specfilm, appropriate wavelength-dependent weights are multiplied. But hdrfilme does not account for this, which results in a color shift. If I’ve misunderstood, please correct me. But anyway, the main point I’m raising is the discrepancy in I’m wondering if this discrepancy is intentional? If it is, then my PR conflicts with the intended design, and I will close it. Thanks again for your time. I may have overlooked something important. Please click here to see my plotting code.import numpy as np
import matplotlib.pyplot as plt
import mitsuba as mi
mi.set_variant("scalar_spectral")
def sample_wavelengths(sensor, samples):
wavelengths = []
weights = []
si = mi.SurfaceInteraction3f()
for sample in samples:
wavelength_4, weight_4 = sensor.sample_wavelengths(si, float(sample))
wavelength, weight = wavelength_4[0], weight_4[0] # Extract the first items
wavelengths.append(wavelength)
weights.append(weight)
return wavelengths, weights
# Sensor without SRF (default behavior)
sensor_dict_default = {
"type": "perspective",
"film": {"type": "hdrfilm", "width": 400, "height": 400},
}
sensor_default: mi.ProjectiveCamera = mi.load_dict(sensor_dict_default)
# Sensor with SRF
# The SRF mimics the default PDF, so it should yield similar results
wavelenths = np.linspace(mi.MI_CIE_MIN, mi.MI_CIE_MAX, 200)
pdf = [mi.pdf_rgb_spectrum(wvl) for wvl in wavelenths]
sensor_dict_srf = {
"type": "perspective",
"film": {"type": "hdrfilm", "width": 400, "height": 400},
"srf": {
"type": "irregular",
"wavelengths": ", ".join(map(str, wavelenths)),
"values": ", ".join(map(str, pdf)),
},
}
sensor_srf: mi.ProjectiveCamera = mi.load_dict(sensor_dict_srf)
# Sample wavelengths from both sensors
samples = np.linspace(0, 1, 100)
wavelengths_default, weights_default = sample_wavelengths(sensor_default, samples)
wavelengths_srf, weights_srf = sample_wavelengths(sensor_srf, samples)
# --------------------------------------------------
# Plotting the results
fig, (ax1, ax2) = plt.subplots(1, 2, figsize=(12, 5))
# Plot default sensor
color = "tab:red"
ax1.set_xlabel("Samples")
ax1.set_ylabel("Wavelengths", color=color)
ax1.plot(samples, wavelengths_default, color=color, label="Wavelengths")
ax1.tick_params(axis="y", labelcolor=color)
ax1.set_title("Without SRF (default PDF)")
ax1_twin = ax1.twinx()
color = "tab:blue"
ax1_twin.set_ylabel("Weights", color=color)
ax1_twin.plot(samples, weights_default, color=color, label="Weights")
ax1_twin.tick_params(axis="y", labelcolor=color)
ax1.legend(loc="upper left")
ax1_twin.legend(loc="upper right")
# Plot SRF sensor
color = "tab:red"
ax2.set_xlabel("Samples")
ax2.set_ylabel("Wavelengths", color=color)
ax2.plot(samples, wavelengths_srf, color=color, label="Wavelengths")
ax2.tick_params(axis="y", labelcolor=color)
ax2.set_title("With SRF (mimics default PDF)")
ax2_twin = ax2.twinx()
color = "tab:blue"
ax2_twin.set_ylabel("Weights", color=color)
ax2_twin.plot(samples, weights_srf, color=color, label="Weights")
ax2_twin.tick_params(axis="y", labelcolor=color)
ax2.legend(loc="upper left")
ax2_twin.legend(loc="upper right")
fig.tight_layout()
plt.show() |
Description
This PR fixes the weight (inverse PDF) calculation in
Sensor::sample_wavelengths()
when sampling through a user-defined SRF (m_srf
), making its behavior consistent with the default path (sample_rgb_spectrum()
).Previously, the implementation directly returned
m_srf->sample_spectrum()
, which maps toRegularSpectrum::sample_spectrum()
orIrregularSpectrum::sample_spectrum()
. The second return value from that function wasm_distr.integral()
, which is not the inverse PDF.With this change, I removed the redundant weight multiplication in
SpecFilm::prepare_sample()
, simplifying the code.Furthermore, rendering with a user-defined SRF in the sensor now produces the correct RGB image. I tested using the following code:
While providing a custom SRF is an uncommon scenario, this fix ensures correctness for those cases.
Checklist
cuda_*
andllvm_*
variants. If you can't test this, please leave below