Skip to content

Return CUSIPs with their check digit and resolve 9-character CUSIPs - #9880

Open
eastagiletracker wants to merge 1 commit into
QuantConnect:masterfrom
eastagiletracker:agile-board/cusip-check-digit
Open

eastagiletracker wants to merge 1 commit into
QuantConnect:masterfrom
eastagiletracker:agile-board/cusip-check-digit

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes returning CUSIPs with their check digit and resolving full 9-character CUSIPs (Fixes #6111). We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/900. You can sign in with your GitHub ID to claim ownership of the project.

Description

The security database only carries the 8-character CUSIP base (e.g. 03783310 for AAPL), and SecurityDefinitionSymbolResolver passed it through unchanged. So Symbol.CUSIP / QCAlgorithm.CUSIP(symbol) returned an 8-character value that is not a valid CUSIP, and QCAlgorithm.CUSIP("037833100") with the real, full CUSIP returned null because the lookup was an exact string match.

This change computes the 9th character with the standard CUSIP "modulus 10 double add double" check-digit algorithm, inside the resolver:

  • CUSIP(Symbol) now returns the full 9-character CUSIP (037833100), appending the check digit when the definition only provides the 8-character base. A definition that already stores 9 characters is returned as-is.
  • CUSIP(string, DateTime) matches on the 8-character base, so both 03783310 (existing callers keep working) and 037833100 resolve to AAPL. A 9-character CUSIP whose check digit is wrong (e.g. 037833101) is not resolved, and an exact string match is still honoured first, so no stored value that previously resolved stops resolving.

SecurityDefinition itself (the raw CSV row) is untouched. The check digits used in the tests were cross-checked against the CUSIP embedded in each security's ISIN (US0378331005 -> 037833100, US38259P7069 -> 38259P706, US38259P5089 -> 38259P508).

Reproduction on current master (80e7843), with the new test cases added before the fix:

dotnet test Tests/QuantConnect.Tests.csproj --filter "FullyQualifiedName~SecurityDefinitionSymbolResolverTests"
  Failed ResolvesCUSIP("037833100",2021,9,9,"AAPL","usa")
    Expected: "AAPL"  But was: null
  Failed ResolvesSymbolToCUSIP(AAPL R735QTJ8XC9X,"037833100")
    Expected string length 9 but was 8.
Failed!  - Failed: 33, Passed: 116, Total: 149

IndustryStandardSecurityIdentifiersRegressionAlgorithm (C# and Python) logs AAPL CUSIP: 03783310 before the change and AAPL CUSIP: 037833100 after it, and both still pass.

How Has This Been Tested?

  • SecurityDefinitionSymbolResolverTests: new cases for 9-character lookups (upper/lower case, across ticker changes GOOCV->GOOG and QQQ->QQQQ->QQQ), wrong check digits, invalid characters and wrong lengths, plus a definition stored with its check digit. The symbol-to-CUSIP expectations now carry the check digit. 149/149 + SecurityDefinitionTests pass (33 of them fail without the change).
  • Ran QuantConnect.Tests.Common.Securities, SymbolTests and QuantConnect.Tests.Algorithm.Algorithm* (17,958 tests) before and after: identical failure set (150 pre-existing failures from Python packages missing in my local environment), no new failures.

Note for review: callers that compared Symbol.CUSIP against the 8-character value will now see 9 characters, which is the behavior #6111 asks for. Lookups by the 8-character base are unchanged.

How this was managed

This work was tracked on a board imported from your repo's issues and pull requests (8,435 stories): the story for it is https://eastagiletracker.com/projects/900/stories/1097977 on https://eastagiletracker.com/projects/900.

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

@arhancanli

Copy link
Copy Markdown

Read the diff, did not run the suite. I recomputed the check digits for the test fixtures with an independent implementation of the mod-10 double-add-double rule and they match: 03783310 -> 0, 38259P70 -> 6, 73935A10 -> 4, 38259P50 -> 8. The 9-character CUSIPs in the new cases are therefore correct (and they agree with the ISINs already in the fixture, e.g. US0378331005 wraps 037833100).

Two things to look at:

  1. CUSIP(string, DateTime) now calls TryGetCUSIPBase on every row inside the FirstOrDefault predicate whenever the input is a valid CUSIP, and that allocates a substring and runs the checksum for each definition before the exact-match comparison can short-circuit. The security definitions list is large, so this turns a cheap string compare per row into a checksum per row on every lookup. Doing the exact comparison first (it already is, via ||) helps only on a hit; on a miss, or for a base lookup, it pays the full cost. Computing the base once per definition when the list is loaded, or indexing by base in a dictionary, would keep lookups O(1).

  2. CUSIP(Symbol) now returns 9 characters where it returned the stored 8-character base before. The remark documents it and the tests were updated, but any algorithm comparing the result to an 8-character string or slicing it will change behaviour silently. If that is intended, it deserves a release note; if not, an overload or a flag keeps the old output.

A small test gap: two definitions that share an 8-character base (a reused or re-issued CUSIP) now match from either the 8- or 9-character form; which one FirstOrDefault returns depends on file order. A case documenting that would pin it.

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.

Implements CheckSum for CUSIP

2 participants