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 socket mode compatibility #232

Merged
merged 14 commits into from
Nov 19, 2024
Merged

Conversation

alchem0x2A
Copy link
Contributor

Nov 18, 2024
Name: Tian Tian, Lucas Timmerman
Changes: (initialization.c, initialization.h, electronicGroundState.c,
include/isddft.h, main.c, main_socket.c, makefile, md.c, relax.c,
socket/.h, socket/.c, tests/Socket/*, CI workflow, doc)

  1. Add socket communication layer and socket submodule
  2. Unifying MLFF and DFT single point calculations to Calculate_Properties function
  3. Update CI workflow for compiling socket code
  4. Update socket mode simple tests
  5. Update documentation for socket mode

@alchem0x2A
Copy link
Contributor Author

@ltimmerman3 please review especially if all your changes to the MLFF codes are incorporated.

Copy link
Contributor

@ltimmerman3 ltimmerman3 left a comment

Choose a reason for hiding this comment

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

Seems mostly okay. Made a suggestion regarding the documentation naming/searching. Need to add declaration for Calculate_Properties to include/electronicGroundState.h

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this is for compatibility with the json API. I'm not sure if it makes more sense to change the file names or just the search string in the API. In fact, I think it makes more sense to allow the api to find files that are agnostic to whether Manual appears first or second. I think the current formulation is a bit brittle as it is entirely possible the file will be renamed again in future commits.

Copy link
Contributor Author

@alchem0x2A alchem0x2A Nov 19, 2024

Choose a reason for hiding this comment

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

I see. The current way API searches the doc is to scan both main Manual.tex and any Manual_*.tex in sub-directories. I'll modify to make sure the main doc tex file is scanned regardless of the name, and keep either one of mlff_manual.tex or Manual_mlff.tex

Copy link
Contributor

Choose a reason for hiding this comment

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

Include function declaration in include/electronicGroundState.h

src/electronicGroundState.c Show resolved Hide resolved
@phanish-suryanarayana phanish-suryanarayana merged commit f6d3a45 into SPARC-X:master Nov 19, 2024
1 check passed
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.

3 participants