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

diff: use index #8930

Merged
merged 1 commit into from
Feb 15, 2023
Merged

diff: use index #8930

merged 1 commit into from
Feb 15, 2023

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Feb 1, 2023

No description provided.

@efiop efiop force-pushed the diff-index branch 2 times, most recently from 7e9f879 to 5bd8667 Compare February 1, 2023 22:41
@codecov
Copy link

codecov bot commented Feb 1, 2023

Codecov Report

Base: 93.34% // Head: 93.34% // No change to project coverage 👍

Coverage data is based on head (18fa1a0) compared to base (18fa1a0).
Patch has no changes to coverable lines.

❗ Current head 18fa1a0 differs from pull request most recent head 9cb65fe. Consider uploading reports for the commit 9cb65fe to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #8930   +/-   ##
=======================================
  Coverage   93.34%   93.34%           
=======================================
  Files         453      453           
  Lines       36510    36510           
  Branches     5278     5278           
=======================================
  Hits        34082    34082           
  Misses       1914     1914           
  Partials      514      514           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@efiop
Copy link
Contributor Author

efiop commented Feb 1, 2023

mnist bench results without any perf analysis:

Screenshot 2023-02-02 at 01 12 54

Most likely view is slowing us down, will double-check iterative/dvc-data#278

EDIT: nope, there is more to it

efiop added a commit to efiop/dvc that referenced this pull request Feb 2, 2023
`data_keys` are similar to `outs`, but they provide more granularity, so we
can use them in `build_data_index` to rebuild index views with filtered
outputs. E.g. when running `dvc diff dir/file`, workspace index should only
have `dir/file` in it.

This also could be used for standalone (non-output) data in the future.

Pre-requisite for iterative#8930
@efiop efiop mentioned this pull request Feb 2, 2023
efiop added a commit to efiop/dvc that referenced this pull request Feb 2, 2023
`data_keys` are similar to `outs`, but they provide more granularity, so we
can use them in `build_data_index` to rebuild index views with filtered
outputs. E.g. when running `dvc diff dir/file`, workspace index should only
have `dir/file` in it.

This also could be used for standalone (non-output) data in the future.

Pre-requisite for iterative#8930
efiop added a commit that referenced this pull request Feb 2, 2023
`data_keys` are similar to `outs`, but they provide more granularity, so we
can use them in `build_data_index` to rebuild index views with filtered
outputs. E.g. when running `dvc diff dir/file`, workspace index should only
have `dir/file` in it.

This also could be used for standalone (non-output) data in the future.

Pre-requisite for #8930
@efiop efiop force-pushed the diff-index branch 2 times, most recently from 58a2af8 to 9cb65fe Compare February 2, 2023 22:06
@efiop efiop force-pushed the diff-index branch 3 times, most recently from 861abf2 to 91970d7 Compare February 15, 2023 04:00
@efiop efiop changed the title [WIP] diff: use index diff: use index Feb 15, 2023
@efiop
Copy link
Contributor Author

efiop commented Feb 15, 2023

Ok. let's run with this. A persistent index will cut it down further and there are additional changes to improve index restoration coming soon for all affected commands.

@efiop efiop marked this pull request as ready for review February 15, 2023 04:39
@efiop efiop merged commit 49b5157 into iterative:main Feb 15, 2023
@skshetry skshetry mentioned this pull request Mar 25, 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.

1 participant