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.

Sign up for GitHub

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

Jump to bottom

142 model download change #150

Merged

ElliottKasoar merged 20 commits into stfc:main from federicazanca:142-model-download-change

Jul 19, 2024

Collaborator

federicazanca commented Jul 10, 2024 •

edited

Loading

Close #142
Changed things:

When downloaded the model file is not saved as a file, just in the database unless the keyword keep_file is used
The existence of the file is checked via "model_hash"
Changed names of the functions to follow convention
Removed filepath attribute, which made it more difficult to share data since it referred to a file path in a local computer. And made some changes in the code related to this removal

federicazanca added 5 commits

July 9, 2024 16:30


          fixed filepath issue, all works for now

7b31e12


          fix test?

6ba4100


          fix test?

ecdf461


          change hashing

329bfc5


          change names of functins

efb9a43

federicazanca requested review from alinelena, ElliottKasoar and oerc0122

July 10, 2024 14:22

federicazanca self-assigned this

Collaborator Author

federicazanca commented Jul 10, 2024

Note: there is the same issue with the filepath in the config file, but actually I ended up never using filepath in the calcjobs or anywhere else, so it's a problem mostly cause it's useless lines of code, but it's usable without issues I think. There are in general some things to change in the config file cause I ended up not using some features, but it's not too urgent.

federicazanca mentioned this pull request

Remove filepath and useless functions in configfile #151

Open


          docs

cc251b6

ElliottKasoar reviewed

View reviewed changes

Member

ElliottKasoar left a comment

Looks good so far.

A few suggestions for tidying up, and a few more general questions where comments here and/or in the code itself my be useful

tests/data/test_model.py Outdated Show resolved Hide resolved

tests/data/test_model.py Outdated Show resolved Hide resolved

docs/source/user_guide/data.rst Show resolved Hide resolved

aiida_mlip/data/model.py Outdated Show resolved Hide resolved

aiida_mlip/data/model.py Outdated Show resolved Hide resolved

aiida_mlip/data/model.py Outdated Show resolved Hide resolved

aiida_mlip/data/model.py Outdated Show resolved Hide resolved

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

federicazanca and others added 3 commits

July 11, 2024 08:55


          Apply suggestions from code review

3ae4b9e

Co-authored-by: ElliottKasoar <[email protected]>


          review fix

40c9252


          add comment

45c50ae

alinelena requested a review from ElliottKasoar

July 11, 2024 09:55

ElliottKasoar reviewed

View reviewed changes

tests/data/test_model.py Outdated Show resolved Hide resolved

alinelena reviewed

View reviewed changes

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

alinelena reviewed

View reviewed changes

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

alinelena reviewed

View reviewed changes

aiida_mlip/calculations/train.py Outdated Show resolved Hide resolved

alinelena reviewed

View reviewed changes

aiida_mlip/calculations/train.py Outdated Show resolved Hide resolved

alinelena reviewed

View reviewed changes

aiida_mlip/data/model.py Outdated Show resolved Hide resolved


          Apply suggestions from code review

147d2f9

Co-authored-by: Alin Marin Elena <[email protected]>
Co-authored-by: ElliottKasoar <[email protected]>

alinelena requested review from ElliottKasoar and alinelena

July 12, 2024 07:57


          name changes

90a6afe

oerc0122 requested changes

View reviewed changes

Collaborator

oerc0122 left a comment

I think fundamentally the change from URL to URI is not what we actually want.

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

aiida_mlip/data/model.py Outdated Show resolved Hide resolved

federicazanca and others added 2 commits

July 17, 2024 10:00


          Update aiida_mlip/calculations/base.py

4eed441

Co-authored-by: Jacob Wilkins <[email protected]>


          fixes and all works

e282c18


          change url to uri

de9800d

alinelena requested a review from oerc0122

July 18, 2024 11:24

oerc0122 reviewed

View reviewed changes

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved

oerc0122 reviewed

View reviewed changes

aiida_mlip/data/model.py Outdated Show resolved Hide resolved

oerc0122 reviewed

View reviewed changes

aiida_mlip/helpers/help_load.py Outdated Show resolved Hide resolved

oerc0122 reviewed

View reviewed changes

tests/data/test_model.py Outdated Show resolved Hide resolved

federicazanca and others added 3 commits

July 18, 2024 15:40


          Update aiida_mlip/calculations/base.py

8a6a004

Co-authored-by: Jacob Wilkins <[email protected]>


          change to URI and fix to qb)

680474d


          fix pre-commit

8e19e78

oerc0122 previously approved these changes

View reviewed changes

ElliottKasoar reviewed

View reviewed changes

aiida_mlip/data/model.py Show resolved Hide resolved

ElliottKasoar reviewed

View reviewed changes

aiida_mlip/calculations/base.py Outdated Show resolved Hide resolved


          Update aiida_mlip/data/model.py

ae650f0

Co-authored-by: ElliottKasoar <[email protected]>

federicazanca dismissed oerc0122’s stale review via

ae650f0

July 18, 2024 16:12

federicazanca added 2 commits

July 18, 2024 17:16


          change folder description

74e1133


          tests path

41257c3

ElliottKasoar approved these changes

View reviewed changes

Member

ElliottKasoar left a comment

Looks good!

alinelena requested review from oerc0122 and ElliottKasoar

July 19, 2024 15:20

ElliottKasoar merged commit 9336017 into stfc:main

8 checks passed

federicazanca deleted the 142-model-download-change branch

July 19, 2024 15:26

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet