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

Allow ULID for MPIDS #928

Merged
merged 5 commits into from
Jan 23, 2024
Merged

Allow ULID for MPIDS #928

merged 5 commits into from
Jan 23, 2024

Conversation

jmmshn
Copy link
Contributor

@jmmshn jmmshn commented Jan 23, 2024

Following the discussion here:
materialsproject/jobflow#519

Added support for ULID: Universally Unique Lexicographically Sortable Identifier

The idea is the UUIDs are random and they cannot be sorted for aggregate document creation.
Since atomate2 will call some emmet.core documents directly we might not always have a chance to parse the task docs and assign MPIDs to everyone.

This PR does the following:

  1. Allows ULID to be used to as MPIDS
  2. The self.parts used for sorting are (ULID_ID, 0) which should enough to help sort the documents.
  3. Updated the logic in StructureGroup to allow for ULID comparison.

@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (4372799) 90.54% compared to head (b8be354) 88.88%.

Files Patch % Lines
emmet-core/emmet/core/mpid.py 81.81% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #928      +/-   ##
==========================================
- Coverage   90.54%   88.88%   -1.67%     
==========================================
  Files         139      109      -30     
  Lines       13203    10054    -3149     
==========================================
- Hits        11955     8936    -3019     
+ Misses       1248     1118     -130     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@munrojm munrojm merged commit d7c8036 into materialsproject:main Jan 23, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants