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

ParticleNet #739

Closed
wants to merge 11 commits into from
Closed

ParticleNet #739

wants to merge 11 commits into from

Conversation

IvanMM27
Copy link
Contributor

ParticleNeT model for Jet Tagging with particle cloud like data used in KM3NeT implemented succesfully in GraphNeT. It uses all utilities from GraphNeT, mainly DynEdgeConv blocks to build its architecture. References:

giogiopg and others added 11 commits May 8, 2024 15:45
New commit including the time zero function in the pulse extractor.
fix , missing in pulse extractor
Including time zero function into the triggered pulse extractor
OrcaNeT model used in KM3NeT
OrcaNeT model imported in gnn init
renamed orcanet to particlenet
Copy link
Collaborator

@RasmusOrsoe RasmusOrsoe left a comment

Choose a reason for hiding this comment

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

@IvanMM27 thank you for adding the ParticleNeT model!

In order to merge this pull request, we need the automatic checks to pass. As you might notice, quite a few of the checks are currently failing. In addition, this PR appears to contain not only changes related to the ParticleNeT model itself, but also code that looks like KM3NeT integration code.

I would suggest that this PR is split up into at least two smaller PR's to reduce the complexity. I.e. One pull request that contains the ParticleNeT model, and then a second pull request that contains the KM3NeT integration.

@IvanMM27 would that be alright with you?

Tagging @giogiopg for completeness

@IvanMM27
Copy link
Contributor Author

@IvanMM27 thank you for adding the ParticleNeT model!

In order to merge this pull request, we need the automatic checks to pass. As you might notice, quite a few of the checks are currently failing. In addition, this PR appears to contain not only changes related to the ParticleNeT model itself, but also code that looks like KM3NeT integration code.

I would suggest that this PR is split up into at least two smaller PR's to reduce the complexity. I.e. One pull request that contains the ParticleNeT model, and then a second pull request that contains the KM3NeT integration.

@IvanMM27 would that be alright with you?

Tagging @giogiopg for completeness

@RasmusOrsoe I think it is better to re-do this pull request again as we want to push only the ParticleNeT model and not any of KM3NeT utilities. Would it be okay if I delete this one and we do another one? Apologies for the misunderstanding.

@giogiopg

@RasmusOrsoe
Copy link
Collaborator

@IvanMM27 thank you for adding the ParticleNeT model!
In order to merge this pull request, we need the automatic checks to pass. As you might notice, quite a few of the checks are currently failing. In addition, this PR appears to contain not only changes related to the ParticleNeT model itself, but also code that looks like KM3NeT integration code.
I would suggest that this PR is split up into at least two smaller PR's to reduce the complexity. I.e. One pull request that contains the ParticleNeT model, and then a second pull request that contains the KM3NeT integration.
@IvanMM27 would that be alright with you?
Tagging @giogiopg for completeness

@RasmusOrsoe I think it is better to re-do this pull request again as we want to push only the ParticleNeT model and not any of KM3NeT utilities. Would it be okay if I delete this one and we do another one? Apologies for the misunderstanding.

@giogiopg

Yes! That is totally fine!

@IvanMM27 IvanMM27 closed this Aug 18, 2024
@giogiopg giogiopg deleted the particlenet branch September 9, 2024 14:58
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