ADR: sequence digest algorithm to be GA4GH digest

[andrewyatz]

No description provided.

[nsheff]

Looks right to me!

[yash-puligundla]

Looks good to me. Link for sha512t24u function points to the comments section. Maybe replace it with "https://journals.plos.org/plosone/article?id=10.1371/journal.pone.0239883"

[andrewyatz]

Good point @yash-puligundla I will need to update that

[sveinugu]

Looks good to me. However, the way the ADR is worded, I think it really also says that we will also include the namespace part of the identifier ("ga4gh:"). I am very much in favor of this, but I am not sure all agree. So we might need to take a separate round on this.

[sveinugu]

Looks good to me. However, the way the ADR is worded, I think it really also says that we will also include the namespace part of the identifier ("ga4gh:"). I am very much in favor of this, but I am not sure all agree. So we might need to take a separate round on this.

Just a few CURIE-related clarifications from the CURIE standard that seems relevant based on the discussions today:

[1]

The prefix part is required to be somehow mappable to a IRI prefix. Concatenating the IRI prefix with the reference part should create a valid IRI, e.g.:

"ga4gh:SQ.whatnot" + {"ga4gh": "https://ga4gh.net/refget/"} = "https://ga4gh.net/refget/SQ.whatnot"

Apart from that, I don't think the CURIE standard defines semantically what the prefix should represent.

[2]

Quoting from the standard:

A host language MAY declare a default prefix value, or MAY provide a mechanism for defining a default prefix value. In such a host language, when the prefix is omitted from a CURIE, the default prefix value MUST be used. Conversely, if such a language does not define a default prefix value mechanism and does not define a set of reserved values, CURIEs MUST NOT be used without a leading prefix and colon.

Basically, the way I read this, is that the prefix part of a CURIE is optional given that the host language defines a default prefix. This is also described in the grammar of CURIEs:

[ [ prefix ] ':' ] reference

I am not sure whether the seqcol standard should be defined as a "host language" though. I assume not. Python, SQL, etc. are host languages, but I don't think data models/APIs are.

[tcezard]

I think we should specify where we intend to use the identifiers. Something along the line of

Suggested change

	The GA4GH identifier will be used as our default sequence identifier instead of MD5. Other identifiers can be provided in a separate array and should not be part of the collection checksum calculation.
	The GA4GH identifier will be used as our default sequence identifier instead of MD5. Other identifiers can be provided in a separate array and should not be part of the collection checksum calculation.
	It will be used to digest:
	- the sequences that are stored in the `sequences` array
	- the canonical representation of arrays of level 2
	- the canonical representation of the sequence collection of level 1

[tcezard]

I don't think we should talk about the prefix ga4gh:SQ in this ADR. First, I don't think we want to use SQ since it is reserved for sequences and second it's not clear that the prefix is part of all the digest that Sequence Collection uses. It should only be used for the level identifier 0.

[nsheff]

There are two issues:

1. What digest algorithm to use
2. How to construct the prefixes of the identifiers that we actually either 2A) Digest; or 2B) Return to users.

I think this ADR should be restricted to the former: this is only about algorithm and not about identifier construction.

We still need to debate the identifier construction question, which I've tried to summarize in issue #37

[tcezard]

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.

[nsheff]

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

[sveinugu]

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

[sveinugu]

Fine by me if we change this to only talk about digests and not include the word "identifier" (or possibly only to explain the difference, if that is needed).

[sveinugu]

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

[nsheff]

ok I think the changes look good... just that are you explicitly wanting to say "preferred" ? Probably saying "will use" or "MUST use" is more accurate...