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.

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

ADR: sequence digest algorithm to be GA4GH digest #31

Merged
merged 7 commits into from
Aug 25, 2023
Merged

ADR: sequence digest algorithm to be GA4GH digest #31

Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b8f54de
Adding sequence identifier ADR
andrewyatz May 11, 2022
139a69f
Updating URL for sha512t24u as raised in PR
andrewyatz Jun 1, 2022
98e9536
Remove mentions of identifiers to digest algorithm
andrewyatz Dec 15, 2022
c6708e1
Merge branch 'master' into adr/sequenceidentifier
nsheff Jan 25, 2023
0c6781f
Decision changed to using sha512t24u
andrewyatz Jul 28, 2023
02798c4
Be stronger about the recommendation
andrewyatz Aug 1, 2023
4c1de0d
Merge branch 'dev' into adr/sequenceidentifier
nsheff Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions docs/decision_record.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,25 @@

[TOC]


## 2023-01-25 - Sequence digest recommendation

### Decision

The GA4GH digest will be used as our default sequence checksum derived identifier instead of an MD5 generated one.

### Rationale

The GA4GH digest was created as part of the [Variation Representation Specification standard](https://vrs.ga4gh.org/en/stable/impl-guide/computed_identifiers.html). This document included a way of creating identifiers to be used with sequences e.g. ACGT results in the identifier `ga4gh:SQ.aKF498dAxcJAqme6QYQ7EZ07-fiw8Kw2`. The scheme uses the [`sha512t24u` function](https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0239883) to create a base64 URL-safe representation of a sha512 digest. Adopting this standard ensures sequence collections remains inline with newer standards within the GA4GH ecosystem.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would clarify the example like this:

Suggested change
The GA4GH digest was created as part of the [Variation Representation Specification standard](https://vrs.ga4gh.org/en/stable/impl-guide/computed_identifiers.html). This document included a way of creating identifiers to be used with sequences e.g. ACGT results in the identifier `ga4gh:SQ.aKF498dAxcJAqme6QYQ7EZ07-fiw8Kw2`. The scheme uses the [`sha512t24u` function](https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0239883) to create a base64 URL-safe representation of a sha512 digest. Adopting this standard ensures sequence collections remains inline with newer standards within the GA4GH ecosystem.
The GA4GH digest was created as part of the [Variation Representation Specification standard](https://vrs.ga4gh.org/en/stable/impl-guide/computed_identifiers.html). This document included a way of creating identifiers to be used with sequences e.g. ACGT is digested as `aKF498dAxcJAqme6QYQ7EZ07-fiw8Kw2` and result in the identifier `ga4gh:SQ.aKF498dAxcJAqme6QYQ7EZ07-fiw8Kw2` once prefixed. The scheme uses the [`sha512t24u` function](https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0239883) to create a base64 URL-safe representation of a sha512 digest. Adopting this standard ensures sequence collections remains inline with newer standards within the GA4GH ecosystem.

Copy link
Member

Choose a reason for hiding this comment

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

@andrewyatz I think we should make this ADR only about algorithm, and not about identifier construction at all, which is handled in #37

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nsheff I agree! We should in that case make sure that we use the term digest and not identifier everywhere.


### Limitations

A GA4GH digest of sequences is not the default identifier used by standards such as CRAM, which uses MD5. We expect sequence collection providers to offer additional checksum arrays to provide compatability with these other formats and to declare their sequence identifier support via service-info.

### Linked issues

- [https://github.com/ga4gh/seqcol-spec/issues/30](https://github.com/ga4gh/seqcol-spec/issues/30)

## 2022-10-05 - Terminology decisions

### Decision
Expand Down Expand Up @@ -239,6 +258,7 @@ We need a formal definition of a sequence collection. The schema provides a mach




## 2021-12-01 - Endpoint names and structure

### Decision
Expand Down