pyroll-project / pyroll-core

PyRoll rolling simulation framework - core library.
https://pyroll.readthedocs.io
BSD 3-Clause "New" or "Revised" License
12 stars 7 forks source link

Non-Deterministioc Errors Raised by Shapely #135

Closed axtimhaus closed 7 months ago

axtimhaus commented 8 months ago

Sometimes there occur non-deterministic errors thrown by shapely because of empty polygons or clipping rectangles. This was especially observed while using the pyroll-pillar-model plugin (currently not published) and dependents.

The reasons for this was identified in the order of root hook evaluation. This order is not deterministic, since root_hooks is a set. Sometimes hooks are evaluated before the first evaluation of the OutProfile.cross_section hook and use therefore the cross section of the in profile passed during initialization. This may conflict with hooks that are evaluated later using the new cross section.

ChRen95 commented 8 months ago

Is also linked to #121 since the model also includes a rounded profile for TreeRollPass class witch would directly alter the cross section.

ChRen95 commented 8 months ago

With further testing also the proposed changes in pr #136 doesn't solve the problem fully. It seems to have lowered the appearance of the bug but it still appears.

axtimhaus commented 7 months ago

The problem may be be solved by fixing the order of root hook evaluation. I have two proposals with benefits and caveats each:

Making root_hooks an OrderedSet from ordered-set

Making root_hook a list

Using one of them directly is not sufficient for my understanding. I would suggest to take the second approach because of the allowed multiplicity, create a subclass and add the following members.

I think we do not need the set operations. I chose the set these days, because I wanted to disallow multiplicity, but from the current point of view this could be beneficial, although it may lead to conflicts between plugins. We should be careful with adding root hooks, only if really needed.

Note, that in a unit, the root hooks are always evaluated in the following order: in profile, out profile, unit. The here proposed changes affect only the order of evaluation within these blocks.

ChRen95 commented 7 months ago

Okay, so we would create a separate PyRolLList (?) like:

class PyRolLList(list):
    def add(self, item):
        self.append(item)

    def insert_after(self, pos, item):
        """Insert an item after a specified position."""
        # Find the index of pos and add 1 to insert after the position
        # If pos is not in the list, this will raise a ValueError
        index = self.index(pos) + 1
        self.insert(index, item)

    def insert_before(self, pos, item):
        """Insert an item before a specified position."""
        # Find the index of pos to insert before the position
        # If pos is not in the list, this will raise a ValueError
        index = self.index(pos)
        self.insert(index, item)

We should then maybe provide additional hooks in the core to solve some errors. Like for example have a defined tension_elongation that is calculated by a plugin and the "overall" calculation would be handled inside the core. Otherwise loading of plugins is indeed very crucial.

axtimhaus commented 7 months ago

I would solely implement it for the root hooks, not for PyRolL as a whole.

Whats the matter with additional hooks? I do not understand the relation.