Clarification on the Subdomain Formulation for F.Reaction

Hi everyone,

I have a quick question on the Reaction class, upon which Trap is based. It appears that when assigning the subdomain, it explicitly imports the 1D subclass of VolumeSubdomain (link):

from festim.subdomain.volume_subdomain import VolumeSubdomain1D as VS1D

However, the 1D version just appears to be a copy of the the main VolumeSubdmain, but with a borders initiated (link):

class VolumeSubdomain1D(VolumeSubdomain):
    """Volume subdomain class for 1D cases.

    Args:
        id (int): the id of the volume subdomain
        borders (list of float): the borders of the volume subdomain
        material (festim.Material): the material of the volume subdomain

    Attributes:
        id (int): the id of the volume subdomain
        borders (list of float): the borders of the volume subdomain
        material (festim.Material): the material of the volume subdomain

    Examples:

        .. testsetup:: VolumeSubdomain1D

            from festim import VolumeSubdomain1D, Material
            my_mat = Material(D_0=1, E_D=1, name="test_mat")

        .. testcode:: VolumeSubdomain1D

            VolumeSubdomain1D(id=1, borders=[0, 1], material=my_mat)
    """

    def __init__(self, id, borders, material) -> None:
        super().__init__(
            id,
            material,
            locator=lambda x: np.logical_and(x[0] >= borders[0], x[0] <= borders[1]),
        )
        self.borders = borders

Therefore, I’m curious if the 1D subclass is actually being used for anything?

Hey @ee-nn

So seems you’ve found a mistake there! So the line from festim.subdomain.volume_subdomain import VolumeSubdomain1D as VS1D is only bringing in the volume subdomain for type hinting, however, this is incorrect as the reaction class does accept any dimension of the subdomain and should be using the standard VolumeSubdomain.

The 1D version of that class does need to exist, though, as when doing a multi-material system, the users need to define the borders of the different materials/subdomains, and we have some checks in place to make sure all the borders cover the whole domain and that every cell is covered.

Hi @jhdark thanks for the clarification! I can open an issue on the repo.