some proposed changes in sph_func, sph_calc ...

[Pankaj Pandey]

I have thought of simplifying creation of new sph_funcs, including vectorized (SIMD, opencl ..) functions.
Here's the basic changes and the rationale.

- There's only one SPHCalc - No SPHBase and SPHEquation (its utility is also questionable in the presence of SPHOperation)
- SPHFunction is base for all funcs. It operates on only onee particle array, w/o any neighbor information reqd (such as position stepping functions). Subclasses can override any of these:
        eval() - operate on arrays (possible to use simd, gpu operations)
        eval_single() - operate on one particle at a time (current situation)
- SPHFunctionParticle(SPHFunction) is base for operations requiring neighbors.Subclasses can override any of these:
        eval_single() - operate on one dest particle (with all src neighbors)
        eval_nbr() - operate on a pair of src and dest particle nbrs
- CSPHFunctionParticle(SPHFunctionParticle) is convenience base class for operations which do separate numerator and denominator calculations. Subclasses can override any of these:
        eval_nbr_csph() - operate on a pair of src and dest particle nbrs and modify numerator and denominator separately. Final result is calculated by dividing nr/dnr computed for all neighbors by calling eval_nbr_csph()

The above distinction also makes it easy to see the type of operation each func performs, rather than maintaining separate information as in test_sph_functions.py.
Another advantage is that the simple operations not using neighbors or denominator are faster.
Also, it this change make SPHCalc feel redundant, it does not do much apart from being a collection of funcs with same dest and multiple srcs. Its functionality could be taken over by SPHOperation

Another change i propose to do is remove the function.py file and the multiple classes there. It has completely redundant information from sph_funcs, and does not provide much convenience over simply using the Function class (which is moved into sph_func.pyx) and passing it the func and args

I've made most of the changes locally but have to test before pushing. Comments are welcome.
Happy Holi to all.

[Kunal Puri]

On Sat, Mar 19, 2011 at 2:41 PM, Pankaj Pandey <pank...@gmail.com> wrote:

I have thought of simplifying creation of new sph_funcs, including vectorized (SIMD, opencl ..) functions.
Here's the basic changes and the rationale.

- There's only one SPHCalc - No SPHBase and SPHEquation (its utility is also questionable in the presence of SPHOperation)
- SPHFunction is base for all funcs. It operates on only onee particle array, w/o any neighbor information reqd (such as position stepping functions). Subclasses can override any of these:
        eval() - operate on arrays (possible to use simd, gpu operations)
        eval_single() - operate on one particle at a time (current situation)
- SPHFunctionParticle(SPHFunction) is base for operations requiring neighbors.Subclasses can override any of these:
        eval_single() - operate on one dest particle (with all src neighbors)
        eval_nbr() - operate on a pair of src and dest particle nbrs
- CSPHFunctionParticle(SPHFunctionParticle) is convenience base class for operations which do separate numerator and denominator calculations. Subclasses can override any of these:
        eval_nbr_csph() - operate on a pair of src and dest particle nbrs and modify numerator and denominator separately. Final result is calculated by dividing nr/dnr computed for all neighbors by calling eval_nbr_csph()

The changes do make the code seem smaller in some places (like sph_calc) but I think we should have some documentation which helps us understand how the looping over particles takes place.
 

The above distinction also makes it easy to see the type of operation each func performs, rather than maintaining separate information as in test_sph_functions.py.
Another advantage is that the simple operations not using neighbors or denominator are faster.
Also, it this change make SPHCalc feel redundant, it does not do much apart from being a collection of funcs with same dest and multiple srcs. Its functionality could be taken over by SPHOperation

Another change i propose to do is remove the function.py file and the multiple classes there. It has completely redundant information from sph_funcs, and does not provide much convenience over simply using the Function class (which is moved into sph_func.pyx) and passing it the func and args

yes, function.py is really double information as far as the sph functions are concerned. Putting Function in sph_func.pyx is a good idea.
 

[Prabhu Ramachandran]

On Saturday 19 March 2011 02:41 PM, Pankaj Pandey wrote:
> I have thought of simplifying creation of new sph_funcs, including
> vectorized (SIMD, opencl ..) functions.
> Here's the basic changes and the rationale.
>
> - There's only one SPHCalc - No SPHBase and SPHEquation (its utility is
> also questionable in the presence of SPHOperation)
> - SPHFunction is base for all funcs. It operates on only onee particle
> array, w/o any neighbor information reqd (such as position stepping
> functions). Subclasses can override any of these:
> eval() - operate on arrays (possible to use simd, gpu operations)

Could this be made eval_array or something more explicit in keeping with
eval_single?

> The above distinction also makes it easy to see the type of operation
> each func performs, rather than maintaining separate information as in
> test_sph_functions.py.
> Another advantage is that the simple operations not using neighbors or
> denominator are faster.

I'm all for simplification. From the zen:
There should be one-- and preferably only one --obvious way to do it.

> Also, it this change make SPHCalc feel redundant, it does not do much
> apart from being a collection of funcs with same dest and multiple srcs.
> Its functionality could be taken over by SPHOperation

True, but would SPHOperation be doing a little too much then? This
change would need more discussion.

> Another change i propose to do is remove the function.py file and the
> multiple classes there. It has completely redundant information from
> sph_funcs, and does not provide much convenience over simply using the
> Function class (which is moved into sph_func.pyx) and passing it the
> func and args

+1

> Happy Holi to all.

Thanks and wish you the same!

cheers,
prabhu
--
Prabhu Ramachandran http://www.aero.iitb.ac.in/~prabhu