Uh oh!
There was an error while loading. Please reload this page.
Introduce indexed ray transfer APIs and tests - #503
Conversation
jacklovell
left a comment
There was a problem hiding this comment.
Very cool. If I understand this correctly, it's a generalisation of the existing ray transfer objects where a callable function replaces the voxel map, and this function returns the voxel index at a given point in space. As well as the 3D application in https://doi.org/10.1063/5.0225703 (which you should cite somewhere in the documentation by the way), I can see the possibility of application to axisymmetric voxels of arbitrary poloidal cross section without having to approximate them with a rectangular grid.
I think a demo would be highly beneficial, as it's hard to see a concrete use case from the docstrings alone. Perhaps you could adapt Vlad's Space Invaders demos to showcase the new tools.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
munechika-koyo
commented
Jul 8, 2026
Thank you for your review!
I will try to work out these ideas with AI assistance for now. |
jacklovell
commented
Jul 8, 2026
Analytically-defined voxels will make a nice demo, agreed. For example, flux-aligned axisymmetric voxels for the generomak equilibrium defined in ( And yes, voxels defined by triangular or tetrahedral meshes would also be a good illustration, showing the ability to extend beyond regular rectilinear grids. |
…yTransferEmitter` class
Mateasek
left a comment
There was a problem hiding this comment.
Hi @munechika-koyo , this is a very nice generalisation of the RT framework! I have one important question to raise.
Changing the indexing function from Function3D to Function6D would allow to pass also direction information which would make the new IRT framwrok applicable to also anisotropic radiation.
| integrator = integrator or IndexedRayTransferIntegrator(step=integration_step) | ||
| super().__init__(integrator=integrator) | ||
| self.index_function = autowrap_function3d(index_function) |
There was a problem hiding this comment.
Making index_function a Function3D makes the IndexedRay framework specific to isotropic radiation. If you made it Function6D which now we have in Cherab, the framework would be generalised to anisotropic applications because you could also pass direction vector components. That is something I'm be very interested in. From the point of view of the code it shouldn't be a large change. What do you think @munechika-koyo?
There was a problem hiding this comment.
That sounds really interesting!
It might also be useful for distributional tomography.
I think we need to add a new API or refactor the existing one to accept Function6D as an index function for the IndexedRayTransferEmitter.emission_function() and IndexedRayTransferIntegrator.integrate() methods.
These changes don't seem straightforward to me, so should we handle them in a separate PR after finalizing this one?
In addition, any good ideas for your demo using Function6D would be really helpful.
There was a problem hiding this comment.
Maybe I'm missing something, but what else do you need to change except the Function3D calls, i.e. in IndexedRayTransferIntegrator.integrate you change <int>index_function(x, y, z) to <int>index_function(x, y, z, dx, dy, dz) and then you need to change the object type and etc.. Maybe I'm missing something.
There was a problem hiding this comment.
Thanks @munechika-koyo for your reaction. It made me look at the Raytransfer framework previously added by @vsnever and the IndexedRaytransfer framework you did in this PR from a broader perspecitve.
Let me now express how I see the framework and what it should do. I think that RayTransfer in general is there to discretise continuous space into one dimensional integer indexes. Then it uses the integration to add a sensitivity to the integer bins.
What does the current RayTransfer framework does? Its specified by a single aspect:
- The mapping between a cartesian space and a 1D array of integers. There are now two distinct mappings for which two sets of classes were made. That is cylindrical and cartesian mappings.
Now what your contribution does is, and correct me if I'm wrong, that it allows user to define the mapping function. This from my point of view generalises the previous approach. Now, we can have a single set of integrator and emitter, which can be used for the old raytransfer functionality if you provide the right discretising mapping functions.
Now I'm going to propose what I think should be done, but please tell me if I'm missing something and I would also like @jacklovell and @skuba31 to think about this, since it would be a major change:
- The
IndexedRaytransferapproach I described above will replace the olderRaytransferbecause essentially it can do exactly the same thing and we should not be doubling our functionality. - The most general indexing function (and replacements for the old framework) together with inverse indexing funcitons will be added to tools to give users the possibility to do some basic and most applications out of the box.
- Extend the indexing function to accept
Function6Dto make this framework even more general.
I'm sorry to be proposing such a major overhaul of this contribution but there wasn't any previous discussion in an issue, so there was no place to do it. I actually had the approach I described above in my head for a long time and I was very happy to see that you actually did it @munechika-koyo, but I think it could be done in a more general and systematic way which would be actually very powerful. If you think about it, it will allow users to apply discretisation in most of the "information dimensions" Raysect passes to the integrator with the ray which position and direction. This would allow to do anisotropic contribution matrices for example. Or you could decide that you implement a discretisation function which uses x, y, z and wavelength (and you just don't use 2 parameters in the 6D function evaluation).What do you think?
| integrator = integrator or IndexedRayTransferIntegrator(step=integration_step) | ||
| super().__init__(integrator=integrator) | ||
| self.index_function = autowrap_function3d(index_function) |
There was a problem hiding this comment.
Maybe I'm missing something, but what else do you need to change except the Function3D calls, i.e. in IndexedRayTransferIntegrator.integrate you change <int>index_function(x, y, z) to <int>index_function(x, y, z, dx, dy, dz) and then you need to change the object type and etc.. Maybe I'm missing something.
skuba31
commented
Jul 23, 2026
First of all, great work @munechika-koyo . I did have a look at the proposed changes with the comments by @Mateasek in mind. At this point I think we should consider again whether we want to include these changes into 1.6 at the expense of postponing the release or whether doing the release as soon as possible is the priority. This discussion could potentially take some manpower that could be dedicated to the release. If not included into 1.6, it could go to development branch and another release be created whenever Matěj finishes his synchrotron model. These two additions would make a logical connection and I think it would justify creation of a release even a short time after 1.6. I like the idea of having a possibility to define any type of index function including those 6D. A possibility to compute contribution matrices for anisotropic radiation would be quite unique. I agree with Matěj that the functionality of the new classes is more general than those currently implemented. I am not sure if I understood the proposal by Matěj correctly, but I think the changes could be managed without changing the public API. If I am reading it correctly, the class structure of the changes proposed by Koyo is: I would propose to use the new indexed classes as a new base for raytransfer and specific raytransfer classes would simply have a index function assigned on creation. So the structure would look like:
If my understanding of the problem is correct, this should include the new functionality while keeping the API basically intact. |
Mateasek
commented
Jul 23, 2026
This is exactly what I
Yes, this is exactly what I had in mind. Whether we include it into 1.6 or not would depend on how long it would take. I personally don't think the changes will be too demanding to implement. Should we create an issue and move this conversation there? |
jacklovell
commented
Aug 20, 2026
Discussion about the design should be done in #310 |
Summary
This PR introduces a general index-function-based ray transfer API by adding indexed emitter and integrator classes, replacing mesh-specific naming with functionality-based naming.
Key Changes
IndexedRayTransferIntegratorIndexedRayTransferEmitterDiscrete3DMesh-backedindex_functionbehaviornumpy.ndindexpatternsUnit Test
test_evaluate_functionIndexedRayTransferEmitterworks correctly withNumericalIntegratorand maps contributions to the correct bins viaindex_function.bins=27. A diagonal ray crosses the volume. The index function maps in-domain points to 0..26 and returns -1 outside.atol=0.001.test_default_integratorIndexedRayTransferEmitteris created without an integrator argument under the same ray/volume setup as above.IndexedRayTransferIntegratorby default, and the resulting spectrum matches the same expected vector (atol=0.001).test_discrete3dmesh_as_index_functionDiscrete3DMeshcan be used as an equivalent index function source.Discrete3DMeshis built from a 4x4x4 vertex grid over 3x3x3 cells; each cube is split into 6 tetrahedra. Cell values follow the same indexing rule as the reference index function.atol=0.001.Executed:
Result:
test_default_integrator: oktest_discrete3dmesh_as_index_function: oktest_evaluate_function: okExample Usage
Compatibility and Risk
Reviewer Notes
Checklist
Benchmark
The appendix in this paper (https://doi.org/10.1063/5.0225703) compared the raytransfer of
Discrete3Dmeshes with that of regular grids, showing that the geometry matrix calculation for rectangular grids' raytransfer was much faster than theDiscrete3Done.