Skip to content

Add a new method called "genre" to the main repo#30

Closed
Jamie001129 wants to merge 0 commit intocharmlab:mainfrom
Jamie001129:main
Closed

Add a new method called "genre" to the main repo#30
Jamie001129 wants to merge 0 commit intocharmlab:mainfrom
Jamie001129:main

Conversation

@Jamie001129
Copy link
Contributor

Except for the new "genre" folder in methods/catalog/ and a test_genre.ipynb, I've also modified the following scripts to integrate the new method.
experiments/experimental_setup.yaml
experiments/run_experiment.py
methods/init.py
methods/catalog/init.py

Copy link
Collaborator

@amirhk amirhk left a comment

Choose a reason for hiding this comment

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

left questions for your review

cc @zkhotanlou

# Load checkpoint
try:
# Try new PyTorch (>= 1.13)
heckpoint = torch.load(full_path, map_location='cpu', weights_only=False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

other than this being a spelling mistake: "heckpoint"

why modify this code? what was the reasoning for having TypeError before?

Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the reason for this extra file? the description at the top does not well-explain/-justify.

Copy link
Collaborator

Choose a reason for hiding this comment

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

again, i'm confused about _newtorch files?

will both *.py and *_newtorch.py files end up in the final submission? which is needed/correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

_newtorch.py are the scripts I created when developing in my python 3.12 environment with newer pytorch. After I moved to the repo's environment some errors due to the difference in pytorch occurred so I made modifications and kept both versions. _newtorch.py files will be removed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If this is a specific model that the GenRe methos works on, please add it to the model catalog

Copy link
Collaborator

Choose a reason for hiding this comment

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

What’s the difference between this implementation and the one in model.py (which was supposed to be named method.py)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The _newtorch.py files are for a later pytorch version in my own environment, they will be removed.

test_genre.ipynb Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

We’re not supposed to have notebook files in the main directory of the repository. To verify that your implementation works correctly, please add unit tests (including assertions) in reproduce.py to ensure that the outputs of your method align with the reported results in the paper.

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