cherab / core

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

Add cylindrical and periodic mappers #388

Closed vsnever closed 1 year ago

vsnever commented 1 year ago

This solves #387 by extending the collection of Cherab mappers with PeriodicMapper#D, VectorPeriodicMapper#D, CylindricalMapper and VectorCylindricalMapper.

The periodic mappers are needed to handle the results of simulations carried out with periodic boundary conditions, while the cylindrical mappers are needed to handle the functions defined in cylindrical coordinates.

For example, with the new mappers a 3D function defined in cylindrical coordinates on a 20° toroidal sector can be mapped with:

CylindricalMapper(PeriodicMapper3D(function, 0, np.pi / 9, 0))

and a vector function can be mapped with:

VectorCylindricalMapper(VectorPeriodicMapper3D(vector_function, 0, np.pi / 9, 0))
jacklovell commented 1 year ago

As we discussed today, these are really coordinate transforms (same dimensionality) rather than mappers (increase dimensionality). The difference is subtle but we should think about how to organise these: I think they should live in a separate submodule under cherab.math.

vsnever commented 1 year ago

I agree, let's move these to cherab.math.transform and rename to CylindricalTransform, PeriodicTransformXD etc.

vsnever commented 1 year ago

The functions are renamed to PeriodicTransform#D, VectorPeriodicTransform#D, CylindricalTransform and VectorCylindricalTransform and moved to cherab.math.transform. The docs are updated to include the new cherab.math.transform sub-module.

jacklovell commented 1 year ago

Looks good. Can you add unit tests for the VectorPeriodic*Transform classes? The scalar classes have tests but the vector classes are missing them.

(By the way, test coverage can be checked by installing recent versions of Cython and Coverage and running ./dev/build.sh --line-profile && coverage run -m unittest && coverage report (with optional coverage html for easier navigation).

vsnever commented 1 year ago

Thanks for the review, @jacklovell. I've added tests for the VectorPeriodicTransform functions.

jacklovell commented 1 year ago

Thanks. Looks good to me now.