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

add "metal" Unit for Lammps #1098

Merged
merged 48 commits into from
Mar 10, 2023
Merged

add "metal" Unit for Lammps #1098

merged 48 commits into from
Mar 10, 2023

Conversation

thangckt
Copy link
Contributor

@thangckt thangckt commented Mar 9, 2023

PR Summary:

add "metal" Unit for Lammps data file

PR Checklist


  • Includes appropriate unit test(s)
  • Appropriate docstring(s) are added/updated
  • Code is (approximately) PEP8 compliant
  • Issue(s) raised/addressed?

Add smiles string conversion using pybel backend (mosdef-hub#1056)
Trim coordinate_transform.py, new features for spin method (mosdef-hub#1054)
Produce n=2 when ports are not capped (mosdef-hub#1069)
add support metal Unit Lammps
Add more notes about unit conversions to the hoomd functions (mosdef-hub#1062)
mbuild/formats/lammpsdata.py Fixed Show fixed Hide fixed
mbuild/formats/lammpsdata.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 9, 2023

Codecov Report

Patch coverage: 71.42% and project coverage change: -0.09 ⚠️

Comparison is base (7941ba4) 89.37% compared to head (bed7ec2) 89.29%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1098      +/-   ##
==========================================
- Coverage   89.37%   89.29%   -0.09%     
==========================================
  Files          61       61              
  Lines        6164     6173       +9     
==========================================
+ Hits         5509     5512       +3     
- Misses        655      661       +6     
Impacted Files Coverage Δ
mbuild/formats/lammpsdata.py 93.15% <71.42%> (-1.22%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@daico007 daico007 left a comment

Choose a reason for hiding this comment

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

The PR looks good overall! Only a few comments. Thanks for your contribution!

mbuild/formats/lammpsdata.py Outdated Show resolved Hide resolved
mbuild/formats/lammpsdata.py Outdated Show resolved Hide resolved
mbuild/formats/lammpsdata.py Outdated Show resolved Hide resolved
mbuild/formats/lammpsdata.py Outdated Show resolved Hide resolved
mbuild/formats/lammpsdata.py Outdated Show resolved Hide resolved
@daico007
Copy link
Member

daico007 commented Mar 9, 2023

Also, I think it worthwhile to add some unit test for the new unit_styles (if you have one available).

thangckt and others added 7 commits March 9, 2023 20:58
Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
Co-authored-by: Co Quach <43968221+daico007@users.noreply.github.com>
@thangckt thangckt requested a review from daico007 March 10, 2023 14:22
@daico007
Copy link
Member

Will merge after tests finished running

@thangckt
Copy link
Contributor Author

thangckt commented May 24, 2023

Dear Prof. @daico007

can you help me with a letter of recommendation?
I will very much appreciate for that

Thank you so much.

@daico007
Copy link
Member

Hi @thangckt, I am currently still a PhD student so I don't know if I have the credential you're looking for, but I would be happy to help in any way that I can.

@thangckt
Copy link
Contributor Author

@daico007
can I reach you on some message platforms?
my fb: thangckt5

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.

2 participants