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

Vs 822 Add documentation for the work that we did on the latest iteration of Delta #8205

Merged
merged 9 commits into from
Mar 14, 2023

Conversation

RoriCremer
Copy link
Contributor

@RoriCremer RoriCremer commented Feb 15, 2023

This includes:

  • the GQ0 --> no call conversion
  • the setting of the max ref block size (already 1000, but need to let the VDS know)

Bonus:
a validation script for the VDS itself

valid

@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

❗ No coverage uploaded for pull request base (ah_var_store@d07e773). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files
@@               Coverage Diff                @@
##             ah_var_store     #8205   +/-   ##
================================================
  Coverage                ?   83.980%           
  Complexity              ?     34807           
================================================
  Files                   ?      2194           
  Lines                   ?    167039           
  Branches                ?     18004           
================================================
  Hits                    ?    140279           
  Misses                  ?     20533           
  Partials                ?      6227           

@RoriCremer RoriCremer marked this pull request as ready for review February 21, 2023 04:09
pip install --force-reinstall hail==0.2.109gi
python3

import hail as hl
Copy link
Collaborator

Choose a reason for hiding this comment

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

hl.init(tmp_dir='...')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this isn't where I want this though, is it?

Copy link
Collaborator

@mcovarr mcovarr Feb 22, 2023

Choose a reason for hiding this comment

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

All of our Hail scripts should do a hl.init(tmp_dir="...") before invoking any other Hail functions, so this would seem to be the spot for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

can I just set it to this?:
hl.init(tmp_dir='gs://fc-secure-fb908548-fe3c-41d6-adaf-7ac20d541375/temp/')

Copy link
Collaborator

Choose a reason for hiding this comment

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

if that's the workspace bucket I think that should be fine

scripts/variantstore/docs/aou/vds/vds_validation.py Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/vds_validation.py Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/Creating a VDS.md Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/Creating a VDS.md Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/Creating a VDS.md Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/Creating a VDS.md Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/Creating a VDS.md Outdated Show resolved Hide resolved
scripts/variantstore/docs/aou/vds/Creating a VDS.md Outdated Show resolved Hide resolved

## Validate it to ensure that it is ready to be shared

Copy the [VDS Validation python script](vds_validation.py) to the notebook environment
Copy link

Choose a reason for hiding this comment

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

nit

Suggested change
Copy the [VDS Validation python script](vds_validation.py) to the notebook environment
Copy the [VDS Validation python script](vds_validation.py) to the notebook environment.

Copy link

@rsasch rsasch left a comment

Choose a reason for hiding this comment

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

one more nit, otherwise 👍🏻

pip install --force-reinstall hail==0.2.109
python3

>>> import hail as hl
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
>>> import hail as hl
>>> import hail as hl
>>> hl.init(tmp_dir='gs://fc-secure-fb908548-fe3c-41d6-adaf-7ac20d541375/hail_temp')

Copy link
Collaborator

@mcovarr mcovarr left a comment

Choose a reason for hiding this comment

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

Approved with the inclusion of hl.init(tmp_dir='gs://blah/...')

@RoriCremer RoriCremer merged commit 3e5a756 into ah_var_store Mar 14, 2023
@RoriCremer RoriCremer deleted the rc-vs-822-GQ0-documentation branch March 14, 2023 17:49
RoriCremer added a commit that referenced this pull request Mar 14, 2023
This was referenced Mar 17, 2023
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.

3 participants