Skip to content

Extending documentation - #218

Merged
pshriwise merged 13 commits into
xdg-org:mainfrom
Waqar-ukaea:extending-docs-introduction
Apr 30, 2026
Merged

Extending documentation#218
pshriwise merged 13 commits into
xdg-org:mainfrom
Waqar-ukaea:extending-docs-introduction

Conversation

@Waqar-ukaea

Copy link
Copy Markdown
Collaborator

Wanted to open a draft PR to make a start on adding some more information/diagrams to the XDG documentation. I will keep adding to this as and when I find the motivation/time

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few thoughts on wording. Should the GPRT submodule have changed as part of this?

Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/design_philosophy.rst Outdated
Comment thread docs/glossary.rst Outdated
@Waqar-ukaea

Copy link
Copy Markdown
Collaborator Author

Just a few thoughts on wording. Should the GPRT submodule have changed as part of this?

Nope, my bad on that one. Lazy application of git add -u probably caused that. Will revert the changes there. This PR should only be docs changes.

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One or two more thoughts here. I think this is a fairly coherent improvement on the current documentation. Would you be alright with merging this and submitting follow-on PRs for other changes that come as motivation/inspiration strikes?

Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/glossary.rst Outdated

.. glossary::

AS

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AS is pretty generic. Can we turn this into two entries: TLAS and BLAS? I think it would prove more useful in the documentation.

Unless perhaps you feel this is too low level to address.

Another option might be to define the term "Acceleration Structure" here, spelling it out. Terms here don't always have to be abbreviations.

@Waqar-ukaea Waqar-ukaea Apr 13, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TLAS and BLAS? I think it would prove more useful in the documentation. Unless perhaps you feel this is too low level to address.

hmmm thats a good idea. I guess it depends on the audience right. We're assuming anyone reading the XDG docs is likely wanting to implement XDG in some kind of downstream simulation code. In that case, I do think its beneficial to have an understanding that there is a two-level structure of trees. And more importantly that the Volumes and Surfaces that they are interacting with correspond to TLAS and BLAS respectively.

Since we also have the global TLAS trees too, the current umbrella term of "Acceleration Structure" probably fails to capture what the RT accel structure is actually doing.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about AS --> Acceleration Structure?

@Waqar-ukaea Waqar-ukaea Apr 16, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about AS --> Acceleration Structure?

Ah yeah I forgot to do the one change that even sparked this conversation and prompted me to add all of this extra detail lol

@Waqar-ukaea

Copy link
Copy Markdown
Collaborator Author

One or two more thoughts here. I think this is a fairly coherent improvement on the current documentation. Would you be alright with merging this and submitting follow-on PRs for other changes that come as motivation/inspiration strikes?

Yep, I think that makes sense. Given the discussion about adding TLAS and BLAS to the glossary would you be okay with me adding some diagrams and a more detailed description of ray tracing and ray tracing acceleration structures as a whole to https://github.com/Waqar-ukaea/xdg/blob/d13968751daab933746a42def56990eff433242f/docs/methods/acceleration_data_structures.rst ?

Something akin to slides 5 & 6 in this presentation is what I had in mind https://docs.google.com/presentation/d/1CA4E30kae5_Bdu1CwrLFjvTwSNLd1qB4/edit?slide=id.p4#slide=id.p4

@pshriwise

Copy link
Copy Markdown
Collaborator

Yep, I think that makes sense. Given the discussion about adding TLAS and BLAS to the glossary would you be okay with me adding some diagrams and a more detailed description of ray tracing and ray tracing acceleration structures as a whole to https://github.com/Waqar-ukaea/xdg/blob/d13968751daab933746a42def56990eff433242f/docs/methods/acceleration_data_structures.rst ?

Something akin to slides 5 & 6 in this presentation is what I had in mind https://docs.google.com/presentation/d/1CA4E30kae5_Bdu1CwrLFjvTwSNLd1qB4/edit?slide=id.p4#slide=id.p4

By all means! Somewhere in there I'm thinking it would be useful to generalize that terminology for Embree where BLAS --> Geometry; TLAS --> Scene.

@pshriwise

Copy link
Copy Markdown
Collaborator

By all means! Somewhere in there I'm thinking it would be useful to generalize that terminology for Embree where BLAS --> Geometry; TLAS --> Scene.

To clarify, you'd like to wait to merge until you've added the TLAS/BLAS discussion?

@Waqar-ukaea

Waqar-ukaea commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator Author

To clarify, you'd like to wait to merge until you've added the TLAS/BLAS discussion?

Yep if that's okay. I'm already partially through writing this up. And I'll re-request a review once that's in place.

@Waqar-ukaea Waqar-ukaea added the documentation Improvements or additions to documentation label Apr 14, 2026
@Waqar-ukaea
Waqar-ukaea requested a review from pshriwise April 14, 2026 13:55
@Waqar-ukaea

Waqar-ukaea commented Apr 14, 2026

Copy link
Copy Markdown
Collaborator Author

I've updated the acceleration_data_structures.rst to better differentiate between TLAS and BLAS (plus added some glossary entries for them) now. One thing to note though is that the mapping of embree types to BLAS vs TLAS isn't necessarily direct since we don't actually currently rely on BLAS instancing that is typically used in TLAS-BLAS structures.

In 5e37e32 I have tried to highlight this with these two mapping tables - not sure if I am getting too in the weeds of the details now with this though. What do you think?

I actually do have another PR up already which switches Embree setup to follow a more explict TLAS-BLAS instance workflow here - #130. My original plan for this PR was to revisit it once I got access to the ANL machines for benchmarking though.

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More thoughts here. It's looking good!

Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/methods/acceleration_data_structures.rst Outdated
Comment thread docs/glossary.rst Outdated

.. glossary::

AS

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about AS --> Acceleration Structure?

@pshriwise pshriwise left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these improvements @Waqar-ukaea!

@Waqar-ukaea

Copy link
Copy Markdown
Collaborator Author

@pshriwise I see you've approved this. Happy to merge?

@pshriwise
pshriwise merged commit e612409 into xdg-org:main Apr 30, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants