SparseZoo V2 Compatibility #1233
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Satrat
requested review from
bfineran,
horheynm,
rahul-tuli,
dbogunowicz and
dsikka
horheynm
previously approved these changes
Sep 8, 2023
bfineran
previously approved these changes
Sep 8, 2023
|
Feeling a little uneasy about the backward compatibility here. @bfineran should we care about it? |
rahul-tuli
reviewed
Sep 11, 2023
| if not zoo_model.sample_originals.files: | ||
| zoo_model.sample_originals.unzip() | ||
| data_originals_path = zoo_model.sample_originals.path | ||
| if zoo_model.sample_inputs is not None: |
There was a problem hiding this comment.
Do we not have sample_originals anymore?
There was a problem hiding this comment.
Some models do not have originals.
horheynm
approved these changes
Sep 11, 2023
sample-originals were uploaded, deleted from the source that triggers the deployment to save to db and blob storage, but it does not trigger deletion. . |
bfineran
approved these changes
Sep 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes two issues related to the SparseZoo V2 upgrade:
sample_originalswas deleted from some models but still being referenced in tests. The tested were updated to usesample_inputsinsteadThe new sparsezooAPI includes subfolder names in thedisplay_namefield ofDirectorieswhen previously it did notbefore(V1):{'display_name': 'config.json', 'file_size': 799, 'model_id': '41dc6dcb-88cd-45f4-ae5e-8c34c0eaef00', 'file_type': 'training', 'url': 'https://api.neuralmagic.com/v2/models/41dc6dcb-88cd-45f4-ae5e-8c34c0eaef00/files/config.json'}after(V2): {'display_name': 'deployment/config.json', 'file_size': 799, 'model_id': '41dc6dcb-88cd-45f4-ae5e-8c34c0eaef00', 'file_type': 'deployment', 'url': 'https://api.neuralmagic.com/v2/models/41dc6dcb-88cd-45f4-ae5e-8c34c0eaef00/files/deployment/config.json'}References to config.json, tokenizer.json and tokenizer_config.json were updated to reflect this change.This is now handled by a change to SparseZoo instead: neuralmagic/sparsezoo@9d9b2baNOTE: A bit unsure about hardcoding the deployment folder name, but since we are already searching for that particular directory name in the sparsezoo repo(seeALLOWED_FILE_TYPES) I think it should be OK to do here too. However, this does remove compatibility with V1The remaining errors are caused by test cases referencing sparsezoo stubs that are deprecated. George is working on replacing these stubs in a separate PR: #1234