Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PillarElement Class #162

Open
ChRen95 opened this issue Jan 17, 2024 · 4 comments
Open

Add PillarElement Class #162

ChRen95 opened this issue Jan 17, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@ChRen95
Copy link
Member

ChRen95 commented Jan 17, 2024

A seperatePillarElementclass should be created to be able, to subdivideDiskElements in width direction.
This would of course make adjustments inside the PillarModel necessary. I'm pretty much up for suggestions about how this is useful and should be a part of the pyroll-core package.

@ChRen95 ChRen95 added the enhancement New feature or request label Jan 17, 2024
@ChRen95 ChRen95 self-assigned this Jan 17, 2024
@ChRen95
Copy link
Member Author

ChRen95 commented Jan 17, 2024

A solution that came up to me would be to add another nested class to DiskElementUnit:

class DiskElementUnit(Unit):
    # ... (existing code)

    class PillarElement(Unit):
        """
        Represents a pillar element unit, a unit used to subdivide a DiskElement in its width direction.
        """

        def __init__(self, parent: 'DiskElement', index: int, **kwargs):
            """
       
            :param kwargs: Additional hook values as keyword arguments to set explicitly.
            """
            super().__init__(label=f"{parent}[{index}]", parent=parent, **kwargs)
            self.length = length

        class Profile(Unit.Profile):
            """Represents a profile in context of a pillar element unit."""

            @property
            def pillar_element(self) -> 'DiskElementUnit.PillarElement':
                """Reference to the pillar element. Alias for ``self.unit``"""
                return cast(DiskElementUnit.PillarElement, self.unit)

    # ... (rest of the existing code)

In my opinion this would make sense, since a pillar_element and a disk_element, share attributes like the length e.g.
`

@ChRen95
Copy link
Member Author

ChRen95 commented Jan 17, 2024

Result of the internal discussions.
IMG_20240117_100018.jpg

@axtimhaus
Copy link
Member

Regarding the PillarElement (and RingElement in the same manner) my main concerns are about computation speed. The current approach in https://github.com/pyroll-project/pyroll-pillar-model with array valued hooks is not as clean as it could be, but it is fast. The count of pillars has a very small impact on speed, whereas the count of disks has heavy impact due to the expensive unit and hook mechanics.

@axtimhaus
Copy link
Member

axtimhaus commented Jan 18, 2024

I have moved all regarding the protocols to a new issue #163. Diskussion here only about the new elements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants