-
Notifications
You must be signed in to change notification settings - Fork 261
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
A Sun and Sky in Mitsuba #1473
base: master
Are you sure you want to change the base?
A Sun and Sky in Mitsuba #1473
Conversation
Dear @matttsss, this is fantastic! Your implementation looks very well-designed, certainly much better than the one in Mitsuba 1 :). Here are some comments from me: Note that the CI fails the tests with the note: Some parts of your implementation (sun-related precomputation in the header file) use double precision numbers in overall single precision code. This will cause many double <-> single implicit conversions to be added, which you probably did not want. Just append an |
One related question: if Python is installed via PyPI/pip, those data files will need to become part of the generated package. Does the installation mechanism work, and does the installed plugin still find them? The file data/srgb.coeff (dynamically generated during the build) might be a good guide on how to get those steps working. |
I took a quick look at the changes in the data repository. You added a number of rendered reference skies, which is great for validation. But they are really large, ~2MB for each image. Over time, such additions make the Mitsuba (and downstream) repos very costly to clone. Could you adapt these tests so that they can work with much less data? -- I am sure that something on the order of 10x less (<200 KiB) would still satisfy the requirement of a correctness check. |
Thank you for your comments,
I was unsure how to proceed with adding the data to the sub-repository, but given the rest of your comments, I will try to generate the data as indicated first before continuing. |
I did not think to test the installation mechanism... Generating the data like the srgb.coeff could be a good idea but the original sampling dataset (for the TGMM) is within a |
No need to generate the TGMM data from CSV. What I meant to say is that the Mitsuba can still find this data after building a wheel and installing it. It would be great if that's also the case for these data files. |
I have worked on reducing the test data size. I also updated the |
Also modify resource path to match that
Also fix gather with index out of bounds
Due to the new `ContinuousDistribution` being built with empty pdf.
Description
This PR introduces a new Sun and Sky emitter based on the Hosek-Wilkie models (sun model, sky model). With the sky's sampling based on Nick Vitsas and Konstantinos Vardis' Truncated Gaussian Mixture Model described in this paper.
The implementation details are resumed in this report, with the issue with the sun's Monte-Carlo weight mentioned in the conclusion solved since.
It makes use of datasets and tests resources that have been provided through:
On plugin instantiation, these datasets are loaded from the files, then are pre-processed (this pre-process step also happens on plugin update). On the contrary to Mitsuba 0.6, this plugin evaluates radiance "on the fly" and does not need an environment map.
The datasets within the binary resource files originate from the reference header files, and were provided as nested C arrays. Those header files are not included but the functions that process them can be found commented at the end of
sunsky.h
along with more detail about how to re-generate the binary files if need be.Testing
Tests were provided within
test_sunsky.py
and check the following:float -> spectrum
conversion is different within both versions of Mitsuba)For the next Chi2 tests, the top of the spherical domain was cropped off since the resolution of the test cannot handle the spike in PDF.
Also, the tests are all done with JIT variants. Although I have checked manually the functionality of the
scalar
variant, since the tests are vectorised, I was unsure how to approachscalar
tests.Checklist
cuda_*
andllvm_*
variants. If you can't test this, please leave below