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

Paper replication package #1

Closed
wants to merge 63 commits into from
Closed

Paper replication package #1

wants to merge 63 commits into from

Conversation

Alphonsce
Copy link
Collaborator

Updates with scripts to replicate results of the paper and README with instructions.

@SpirinEgor SpirinEgor changed the title python code and bash scripts to replicate results and readme with instructions Paper replication package Apr 23, 2024
Copy link
Member

@SpirinEgor SpirinEgor left a comment

Choose a reason for hiding this comment

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

  1. For all notebooks clean cells outputs and from the beggining (run all)
  2. Run black . -l 120 и isort .

pytest==7.4.3
pytest-subtests==0.11.0
isort==5.12.0
git+https://github.com/deepvk/metr@for_paper
Copy link
Member

Choose a reason for hiding this comment

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

requirements should contain only top-level dependencies, like torch or wandb. No need to pip freeze your environment here

Copy link
Member

Choose a reason for hiding this comment

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

Also, this is dev dependencies, it should contains packages for developments, e.g. black.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in branch code_fixes, which I plan to merge with this branch for_paper and then this one with master

@@ -0,0 +1,155 @@
# Licensed under the Apache License, Version 2.0 (the "License");
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that we need this file

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in code_fixes branch

@@ -0,0 +1,29 @@
# I call it a 0.1.0 version
Copy link
Member

Choose a reason for hiding this comment

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

Remove please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in code_fixes branch

import sys
import os

sys.path.append(os.path.dirname(os.path.abspath(__file__)))
Copy link
Member

Choose a reason for hiding this comment

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

This is also unnecessary

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed in code_fixes

Comment on lines +3 to +4
def check(pipeline: DiffusionPipeline, model_hash: str):
pass
Copy link
Member

Choose a reason for hiding this comment

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

What is the point? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed useless files: current code in code_fixes

@Alphonsce Alphonsce closed this May 9, 2024
@Alphonsce Alphonsce reopened this May 9, 2024
@Alphonsce Alphonsce closed this May 17, 2024
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.

2 participants