Labeling of coordinates in cylindrical (and spherical) meshes

[mhvwerts]

PyFVTool systematically uses the labels x, y and z to represent the first, second resp. third coordinate, irrespective of the mesh geometry (Cartesian, cylindrical, spherical). This is important especially for internal code consistency and keeping the code base simple and free from Pythonic wizardry which may generate head-aches. Once used to it, this is OK, and one writes, e.g.,

import pyfvtool as pf
msh = pf.createMeshCylindrical2D(Nr, Nz, Lr, Lz)
rr = msh.cellcenters.x
zz = msh.facecenters.y

However, it would still be nice, one day, to consider the possibilities for adapting the programming interface to cellcenters and facecenters (and createMeshCylindrical etc.) such that the user would only see and use r, z (and theta, phi) when working with cylindrical or spherical meshes. This would improve the readability of the user's code and even avoid some errors. Internally, PyFVTool would continue to use labels x, y and z, but this would be invisible to the user, and translated by the functions and methods for creating and accessing meshes.

I have some vague ideas how this could be achieved in a relatively simple manner, but already wondering if @simulkade thinks if this is somewhere remotely feasible and desirable, and what others would think of such a new feature.

[simulkade]

Funny you mentioned it @mhvwerts. I was working on a software that switches between Cartesian and Cylindrical coordinates, with some strange BCs changing with time. I found myself looking at the source code a couple of times because I was not sure about what x and y means, mostly for xvalue and yvalue of FaceVariables. I think it is a good idea to find a simple solution for it. I had a vague idea for defining subclasses, but cannot test them before the mid-September. All in all, I must admit it is a very desirable feature to have.

[mhvwerts]

Oh yeah, xvalue and yvalue of FaceVariables, I forgot to mention those... I am experimenting with convection-diffusion in 2D cylindrical (typical Taylor dispersion experiment) as this is a relevant test case and it took some trial-and-error to set up the convective term (velocity component in z direction, magnitude dependent on r).

Let's give it time. IMO, implementing this feature should be done in one run, and probably best that I make sure that we have a decently working pytest (#12) before such an undertaking as it may break stuff.

[simulkade]

Absolutely agree. Let's keep it as an open issue. I usually do some python-therapy when I get to busy with writing tasks. If I come up with a good idea, I will write it here.

[simulkade]

I just tried Co-pilot and it suggests using property decorators:

class MyClass:
    def __init__(self, xvalue, yvalue):
        self.xvalue = xvalue
        self.yvalue = yvalue
    
    @property
    def rvalue(self):
        return self.xvalue
    
    @rvalue.setter
    def rvalue(self, value):
        self.xvalue = value
    
    @property
    def zvalue(self):
        return self.yvalue
    
    @zvalue.setter
    def zvalue(self, value):
        self.yvalue = value

The same thing can be done with a subclass. We also need to define subclasses of FaceVariable for different coordinates and then use the decorators.

[mhvwerts]

That's an interesting suggestion. It may be wise, perhaps necessary, to combine this with renaming the 'internal' x, y, z to _x, _y, _z (or x_ ...). I see two reasons to do so:

1. Avoiding a conflict for 2D cylindrical, where the name z becomes ambiguous: 'exterior/user' z refers to 'internal' y, but what happens with the 'internal' z? (I am not sure if the internal z exists in 2D cylindrical, I know it is of no use. The conflict will surely exist for 3D cylindrical.)
2. Readability of the PyFVTool code: clear identification whether a coordinate label refers to the 'user' labeling or to the 'internal' labeling (this may save headaches later on)

[simulkade]

I agree. They cannot be hidden from the user but the underscore can help. This z being y in cylindrical coordinate is one of those stupid thing I realized later like all other stupid things I have done in my life :-))

[mhvwerts]

The re-labeling strategy might consist of a well-targeted and supervised find-and-replace of .x .y .z by ._x ._y ._z followed by fixing any errors that occur through adapting/providing subclasses or changing the user code. Do you think that that might work?

[simulkade]

It will work but needs some work. I wish Python would let us define private attributes but it is not the case.

[mhvwerts]

The underscore at the beginning of the attribute name should be enough a deterrent for the casual user not to touch these attributes.