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.

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

Jump to bottom

Refine IPPM graph datastructure #362

Open
caiw opened this issue Sep 6, 2024 · 1 comment · May be fixed by #391
Open

Refine IPPM graph datastructure #362

caiw opened this issue Sep 6, 2024 · 1 comment · May be fixed by #391
Assignees
@caiw
Labels
👷 design standards ❓ discussion needed Extra discussion is needed before work can commence IPPM generation ⚙️ refactor Changes which relate to code structure rather than functionality

Comments

@caiw
Copy link
Member

caiw commented Sep 6, 2024 •
edited
Loading

There is some duplication in the IPPM code, stemming from it's original design against the API. This means that some graphing code, and some classes which define hexels, spikes, pairings, etc, are similar, and could probably be consolidated. Also, I (@caiw) get a little confused when reading through how the graphs datastructure is currently built and traversed (it's not wrong or bad, I just wonder if it could be more intuitively defined).

Low priority, but I've been wondering about combining it into a networkx.DiGraph, perhaps whose nodes are literal Transform objects (see #65).

We can discuss pros and cons on this issue.
@caiw caiw added ❓ discussion needed Extra discussion is needed before work can commence 👷 design standards ⚙️ refactor Changes which relate to code structure rather than functionality IPPM generation labels Sep 6, 2024
This was referenced Sep 6, 2024
Closed
Merged
Open
@caiw caiw self-assigned this Sep 14, 2024
@neukym neukym mentioned this issue Sep 14, 2024
Closed
@neukym
Copy link
Member

neukym commented Sep 14, 2024

perhaps whose nodes are literal Transform objects

I think this should this be "whose edges are literal Transform objects?"

This was referenced Sep 21, 2024
Open
Merged
Merged
Closed
@caiw caiw linked a pull request Oct 25, 2024 that will close this issue
Draft
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@caiw caiw

Labels
👷 design standards ❓ discussion needed Extra discussion is needed before work can commence IPPM generation ⚙️ refactor Changes which relate to code structure rather than functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

IPPM graph refactor kymata-atlas/kymata-core
2 participants
@neukym @caiw