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

Adding correction for multigraph aspect #6

Merged
merged 6 commits into from
Aug 16, 2022
Merged

Conversation

Forbu
Copy link
Contributor

@Forbu Forbu commented Jun 1, 2022

No description provided.

@ksbeattie
Copy link
Member

@Forbu, can you provide more details on what these changes fix or add?

@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Jun 7, 2022
@ksbeattie ksbeattie requested review from pn51 and bbartoldson June 7, 2022 20:08
@Forbu
Copy link
Contributor Author

Forbu commented Jun 8, 2022

@ksbeattie Pardon me for the lack of details !
I was trying to make the multigraph MGN work for my own project and I found your cool repo (which work really well).
So I make some modifications to make the "multigraph" part work (it wasn't working previously).

  1. I made some correction on some class initializator because we have to initiate the nn module class : I just uncomment the "super" initializator.
  2. Also they was an issue because we didn't have the EdgeProcessor in a module list : the EdgeProcessor list was an module attribute of another module but without being in a moduleList ... I made the correction on that.
  3. Also they was an issue when all the nodes were not pass in the edge_index / edge attribute : it could generate an error ... I made the correction by passing the number of edge in the scatter sum (dim_size param) inside the node processor loop.

@ksbeattie ksbeattie requested review from pn51 and bbartoldson and removed request for pn51 and bbartoldson July 19, 2022 20:25
@ksbeattie
Copy link
Member

@pn51 a/o @bbartoldson can you comment on this?

@pn51
Copy link
Contributor

pn51 commented Jul 27, 2022

I'll take a look.

@ksbeattie
Copy link
Member

@pn51 are we waiting on another review from @bbartoldson or is this ok to merge now?

@ksbeattie ksbeattie merged commit 9c7b143 into CCSI-Toolset:main Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority:Normal Normal Priority Issue or PR
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants