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

Refactor SCC expansion #118

Merged
merged 1 commit into from
Jun 5, 2024
Merged

Refactor SCC expansion #118

merged 1 commit into from
Jun 5, 2024

Conversation

daemontus
Copy link
Collaborator

Should fix the SCC expansion bug, plus perform some other clean up in that Python module.

Copy link

Coverage

Coverage Report
FileStmtsMissCoverMissing
biobalm
   _pint_reachability.py615018%24, 40–54, 69–93, 101–146
   control.py1141488%107, 119, 125, 129, 134, 143–159, 477, 480, 493
   interaction_graph_utils.py52688%11–13, 151–152, 206–207
   petri_net_translation.py1491193%22–26, 79, 136, 308–309, 333–334, 343, 452
   space_utils.py1322085%26–28, 104–110, 133–139, 347–350, 414, 462
   succession_diagram.py3706582%6, 119, 208–213, 226, 273–280, 384–391, 408–409, 419, 425, 541, 628–634, 750, 753, 871–889, 921, 931, 934, 974, 981, 1032, 1046, 1126, 1278, 1289, 1297, 1325, 1340, 1352, 1357, 1363
   symbolic_utils.py32584%10, 39–44, 100, 128
   trappist_core.py1832785%14–18, 55, 57, 92, 215, 217, 219, 247–250, 254–256, 276–282, 340, 342, 372, 420, 422
biobalm/_sd_algorithms
   expand_attractor_seeds.py60788%6, 28, 42, 109–114, 119
   expand_bfs.py28196%6
   expand_dfs.py30197%6
   expand_minimal_spaces.py37295%6, 31
   expand_source_SCCs.py1111686%11–13, 50, 69, 77, 82, 103, 112, 120, 131, 140, 143, 167, 179, 242–243
   expand_to_target.py31390%6, 38, 43
biobalm/_sd_attractors
   attractor_candidates.py2649066%13–15, 26–27, 93, 101, 107–108, 130, 152, 187, 193–204, 223, 239–320, 325, 329, 335, 341, 352, 376, 381, 385, 391, 393–431, 504, 575–576, 677
   attractor_symbolic.py1141686%6–7, 75, 88–92, 103, 112, 144, 179, 191–193, 202, 230, 236
TOTAL185333482% 

Tests Skipped Failures Errors Time
356 0 💤 0 ❌ 0 🔥 54.006s ⏱️



def expand_source_SCCs(
sd: SuccessionDiagram,
expander: ExpanderFunctionType = expand_bfs,
check_maa: bool = True,
check_maa: bool,
Copy link
Owner

Choose a reason for hiding this comment

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

Any reason check_maa isn't optional anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah, I forgot to put it back. I was playing with it when I started using the function recursively because it seemed like the flag wasn't propagating correctly. I'll put it back.

f" > [{node_id}] Found {len(source_scc_diagrams)} sub-diagrams while expanding node."
)

# If there are no source SCCs, this node is a fixed-point and we can save it
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would having no source SCCs make the node a fixed point?

Copy link
Collaborator

@kyuhyongpark kyuhyongpark left a comment

Choose a reason for hiding this comment

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

I proof-read the code to my best ability. Everything looks very nice.

@daemontus daemontus mentioned this pull request Jun 4, 2024
@jcrozum jcrozum merged commit 6eea992 into main Jun 5, 2024
8 checks passed
@jcrozum jcrozum mentioned this pull request Jun 5, 2024
@daemontus daemontus deleted the scc-refactor branch September 18, 2024 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants