graphnet-team / graphnet

A Deep learning library for neutrino telescopes
https://graphnet-team.github.io/graphnet/
Apache License 2.0
90 stars 92 forks source link

ISeeCube implentation #676

Closed ChenLi2049 closed 5 months ago

ChenLi2049 commented 6 months ago

This model requires torchscale 0.2.0, which can be installed by:

pip install torchscale==0.2.0

I checked Install — graphnet documentation, more information on IceTray would be much appreciated.

RasmusOrsoe commented 6 months ago

@ChenLi2049 Thanks for this!

Quite a few of the tests fail for this PR; it appears to be mainly related to code formatting. Could you please install pre-commit (see contribution guide) and have it format the code?

RasmusOrsoe commented 6 months ago

@ChenLi2049 This still appears to be an issue (see https://github.com/graphnet-team/graphnet/actions/runs/8316406836/job/22883128660?pr=676).

ChenLi2049 commented 6 months ago

Other than Function __init__ has 9 arguments (exceeds 4 allowed). Consider refactoring., all the checks passed. I think it's good to go. :)

RasmusOrsoe commented 6 months ago

@ChenLi2049 thank you for dealing with the unit tests - glad to see you figured it out! Could you please update to the latest main and resolve the merge conflicts from #680?

ChenLi2049 commented 6 months ago

If the modification of FourierEncoder is ok, it's ready to merge.

ChenLi2049 commented 5 months ago

Hi @RasmusOrsoe, I am not familar with https://github.com/graphnet-team/graphnet/actions/runs/8707466377/job/23882513254?pr=676, can you help me with it?

RasmusOrsoe commented 5 months ago

@ChenLi2049 Hey - sorry for this. I have proposed a fix in #695 . Once it's merged into main, you should be able to resolve this error by updating your branch.

RasmusOrsoe commented 5 months ago

@ChenLi2049 issue is now fixed in main

ChenLi2049 commented 5 months ago

Hi @ArturoLlorente , what kind of changes? I'm glad to help.

ChenLi2049 commented 5 months ago

Hi @ArturoLlorente , the failed check is failed when installing packages. It seems like the wrong package is downloaded?

ChenLi2049 commented 5 months ago

OK, now other than "__init__ has more than 4 arguments", all the checks passed. I think it's ready to merge. :)