cherab / core

The core source repository for the Cherab project.
https://www.cherab.info
Other
44 stars 24 forks source link

Add 'units' attribute to RayTransferPipeline#D #413

Closed vsnever closed 1 year ago

vsnever commented 1 year ago

This PR fixes #412 by adding units attribute to RayTransferPipeline#D, which can be either 'power' or 'radiance', similar to the Power and Radiance pipelines. This attribute determines whether the ray transfer matrix is multiplied by the sensitivity of the detector ('power') or not ('radiance').

The RayTransferPixelProcessor class is now split into two subclasses: PowerRayTransferPixelProcessor and RadianceRayTransferPixelProcessor.

The RayTransferPipeline#D classes now inherit from both Pipeline#D classes and the new RayTransferPipelineBase class.

The tests for RayTransferPipeline#D are added.

skuba31 commented 1 year ago

Hi Vlad, thanks for handling this. It is one of the things I had to figure out when I was starting to use the ray transfer matrix over the single line integrated geometry/contribution matrices I was calculating with my own code.

I suppose it is mostly personal preference but the name units feels to be slightly misleading as the units of ray transfer matrix are not 'radiance' in case of [m] or 'power' for [m3 sr].

I personally would thinking about such property as something like quantity as the power or radiance is the result obtained after the matrix multiplication with discretised "power density" (W m-3sr-1). If the name units is to be used, I would set the accepted values to be 'length' and either of 'throughput' or 'volume-angle' or 'volume-sr'...

vsnever commented 1 year ago

Hi Jakub, the attribute is badly named indeed, thanks. However, I'd like to keep these power/radiance options because they match the Raysect pipelines and are easy to remember. I suggest renaming the units attribute to kind. The quantity is better than units, but still the power density or the radiance are the quantities of the matrix product, not the matrix itself.

skuba31 commented 1 year ago

I agree, kind is better name for the attribute.

vsnever commented 1 year ago

I agree, kind is better name for the attribute.

Done.