-
Notifications
You must be signed in to change notification settings - Fork 593
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
Fix full dataset download after partial #4681
Fix full dataset download after partial #4681
Conversation
Hello. You may have forgotten to update the changelog!
|
… of github.com:PennyLaneAI/pennylane into sc-42605-download-dataset-works-after-download-partial
Heads up, this will be slow on large datasets until #4674 is merged |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4681 +/- ##
==========================================
- Coverage 99.64% 99.63% -0.01%
==========================================
Files 377 377
Lines 34002 33754 -248
==========================================
- Hits 33881 33632 -249
- Misses 121 122 +1
☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! just a few things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great! you should add yourself to the contributor list too 😄
EDIT: I see you added yourself in the other PR, so nvm :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
Context:
Link to shortcut story
If you call load and download only a partial dataset (eg. load("qchem", molname="H2", attributes=["hf_state"])), then try to load the full dataset (call load() again but without the attributes kwarg), it will not download the rest of the dataset. This is because if _download_dataset sees that there is already a file for some dataset, it assumes it has the data it needs. You can add force=True to ensure it's re-downloaded, but you shouldn't need to.
Description of the Change:
If all attributes are requested in
qml.load()
(e.gattributes
isNone
), and the dataset already exists locally,_download_partial()
will be used to download the missing attributes. Ifforce
is specified, the existing attributes in the local dataset will be overwritten._download_full()
will only be called if the dataset does not exist at all locally.Benefits:
Mixed calls to
qml.load()
work as expected.Possible Drawbacks:
Related GitHub Issues: