Skip to content

Some bug fixes in trimming and isolated dft function added #10

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Some bug fixes in trimming and isolated dft function added #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dftlib/storage/dft_gates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ def compare(self, other, respect_ids):
if not super().compare(other, respect_ids):
return False

if not self.compare_successors(other, ordered=True, respect_ids=respect_ids):
if not self.compare_successors(other, ordered=True, respect_ids=True):
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this change necessary?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was made in collaboration with Dr. Falak to address specific scenarios encountered during fault tree simplification. Unfortunately, I don’t recall the exact details now, but the modification was likely introduced to prevent the function from getting stuck in recursion under certain conditions.

return False

return self.inclusive == other.inclusive
Expand Down
11 changes: 7 additions & 4 deletions dftlib/transformer/rewrite_rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,9 @@ def try_replace_fdep_by_or(dft, fdep):
trigger = fdep.trigger()
dependent = fdep.dependent()[0]

if dependent.relevant:
return False

# Check if both trigger and dependent are part of the top module
top_module = dft.get_module(dft.top_level_element)
if trigger.element_id not in top_module or dependent.element_id not in top_module:
Expand Down Expand Up @@ -577,9 +580,6 @@ def try_remove_superfluous_fdep(dft, fdep):
trigger = fdep.trigger()
dependent = fdep.dependent()[0]

# Trigger is single parent of dependent (apart from fdep)
if len(dependent.parents()) > 2:
return False
Comment on lines -580 to -582
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be kept. If the dependent event has another parent, the dependent event is still relevant for this other event and therefore the FDEP cannot be removed.
Or did I overlook something?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not entirely sure about the original reasoning, as it was done some time ago. It’s possible that I considered the check unnecessary at the time and decided to remove it. However, your point makes sense — if a dependent event has another parent, it would still be relevant, and the FDEP should not be removed. It’s worth revisiting this with that in mind.

if trigger not in dependent.parents():
return False

Expand Down Expand Up @@ -628,10 +628,13 @@ def try_remove_fdep_successors(dft, fdep):
trigger = fdep.trigger()
dependent = fdep.dependent()[0]

if dependent.relevant:
return False

# Dependent has single parent (apart from fdep)
if len(dependent.parents()) > 2:
return False
parent = dependent.parents()[0] if not isinstance(dependent, dft_gates.DftDependency) else dependent.parents()[1]
parent = dependent.parents()[0] if not isinstance(dependent.parents()[0], dft_gates.DftDependency) else dependent.parents()[1]
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Thanks!


# Trigger has the same parent
if trigger not in parent.children():
Expand Down
77 changes: 68 additions & 9 deletions dftlib/transformer/trimming.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@

import dftlib.storage.dft_gates as dft_gates

def get_releated_parents(current, visited, related_parents_visited):
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that this function is used to also perform an "upwards" search in order to find all connected elements. Could we not extend the original function by also considering SPARES?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, yes. The main reason I wrote this function separately was to ensure that no parent elements are missed, even if they are not directly connected. I needed a recursive approach to trace and include all relevant parent elements. That’s why I implemented it as a standalone function, rather than modifying the original one.

related_parents = []

if isinstance(current, dft_gates.DftDependency) or isinstance(current, dft_gates.DftSeq) or isinstance(current, dft_gates.DftMutex) or isinstance(current, dft_gates.DftSpare):
if current.element_id not in visited:
related_parents.append(current)
if current not in related_parents_visited:
related_parents_visited.append(current)
for parent in current.parents():
if (isinstance(parent, dft_gates.DftDependency) or isinstance(parent, dft_gates.DftSeq) or isinstance(parent, dft_gates.DftMutex) or isinstance(parent, dft_gates.DftSpare) ) and parent.children()[0] == current:
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the significane of the first child? Why is there no similar check for the other children needed?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this change around two years ago, so I don’t recall the exact scenarios or reasoning in full detail. However, it was likely intended to prevent unnecessary recursion, assuming that the first child had already been processed. This adjustment was part of a quick fix implemented under time constraints, and although it worked correctly in our use cases, I cannot guarantee its correctness across all scenarios. That’s why I’m sharing it with you — so you can analyze it more thoroughly and decide whether it should be incorporated, considering all relevant aspects.

continue
related_parents += get_releated_parents(parent, visited, related_parents_visited)

return related_parents

def trim(dft):
"""
Expand All @@ -25,15 +39,60 @@ def trim(dft):
# Element is necessary
del unused[current.element_id]

# Continue search
if current.is_be():
# Add possible SEQ or FDEP gates
for parent in current.parents():
if isinstance(parent, dft_gates.DftDependency) or isinstance(parent, dft_gates.DftSeq) or isinstance(parent, dft_gates.DftMutex):
if parent.element_id not in visited:
queue.append(parent)
visited.add(parent.element_id)
elif current.is_gate():
related_parents = get_releated_parents(current, visited, [])
for related_parent in related_parents:
if related_parent.element_id not in visited:
queue.append(related_parent)
visited.add(related_parent.element_id)

if current.is_gate():
# Add children of gate
for child in current.children():
if child.element_id not in visited:
queue.append(child)
visited.add(child.element_id)

assert len(unused) == len(dft.elements) - len(visited)

# Remove unused elements
trimmed = False
for element in unused.values():
if not element.relevant:
dft.remove(element)
trimmed = True
return trimmed


def isolated(dft):
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the purpose of this function?
As far as I can see it only differs from the trim function in one line (only applying get_related_parents if current is not the TLE). Could the trim function not be used instead and we add a check for the TLE in get_related_parents?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're right — there is no major difference between the two functions. The isolated function was created to extract an isolated subtree, which differs slightly from the standard trimmed tree. Specifically, in the isolated version, we do not consider the top-level event's failure due to triggers, sequences, or other control behavior. Instead, we focus solely on failures caused by its children.

This function is not intended to be part of the simplification process. We use the isolated DFT structure in specific criticality analyses, such as computing the Birnbaum Index (BI). For convenience, I placed this function alongside the trimming logic since it is structurally similar, with only minor modifications. I chose not to alter the existing trim function's interface to avoid impacting existing functionality.

Feel free to refactor or integrate it as you see fit.

"""
Trim parts of the DFT in place which do not contribute to the top level element.
:param dft: DFT which will be modified.
:return: True iff elements were trimmed.
"""

# List of possible unused elements to trim
unused = {elem_id: element for elem_id, element in dft.elements.items()}
visited = set()

# Perform breadth-first search to find all necessary elements
queue = deque()
queue.append(dft.top_level_element)
visited.add(dft.top_level_element.element_id)
while len(queue) > 0:
current = queue.popleft()

if current.element_id in unused:
# Element is necessary
del unused[current.element_id]

if current != dft.top_level_element:
related_parents = get_releated_parents(current, visited, [])
for related_parent in related_parents:
if related_parent.element_id not in visited:
queue.append(related_parent)
visited.add(related_parent.element_id)

if current.is_gate():
# Add children of gate
for child in current.children():
if child.element_id not in visited:
Expand Down
Loading