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

Support chemical JSON #50

Merged
merged 5 commits into from
Jul 26, 2022
Merged

Support chemical JSON #50

merged 5 commits into from
Jul 26, 2022

Conversation

awvwgk
Copy link
Member

@awvwgk awvwgk commented Jul 26, 2022

Enable support for reading chemical JSON in xtb, dftd4, etc., chemical JSON is beside CML the only format supported in Avogadro 2 for saving structures.

  • implement basic reader for chemical JSON
  • implement writer for chemical JSON
  • test if chemical JSON produced by mctc-lib round-trips
  • document implemented format

Specs:

Impl.:

- implement basic reader for chemical JSON
@awvwgk
Copy link
Member Author

awvwgk commented Jul 26, 2022

@ghutchis Is there a more up-to-date specification of chemical JSON than https://wiki.openchemistry.org/Chemical_JSON?

@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #50 (9c134f3) into main (f02b590) will decrease coverage by 1.01%.
The diff coverage is 58.57%.

@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
- Coverage   69.80%   68.78%   -1.02%     
==========================================
  Files          60       64       +4     
  Lines        7616     8403     +787     
  Branches     2232     2503     +271     
==========================================
+ Hits         5316     5780     +464     
- Misses        739      873     +134     
- Partials     1561     1750     +189     
Impacted Files Coverage Δ
test/main.f90 31.03% <ø> (ø)
src/mctc/io/write/cjson.f90 23.97% <23.97%> (ø)
test/test_write_cjson.f90 53.84% <53.84%> (ø)
test/test_write.f90 56.41% <54.54%> (-0.20%) ⬇️
test/test_read_cjson.f90 63.14% <63.14%> (ø)
src/mctc/io/read/cjson.F90 66.94% <66.94%> (ø)
test/test_read.f90 93.23% <95.23%> (+0.32%) ⬆️
src/mctc/io/filetype.f90 45.65% <100.00%> (+1.20%) ⬆️
src/mctc/io/read.f90 51.35% <100.00%> (+1.35%) ⬆️
src/mctc/io/write.f90 48.57% <100.00%> (+1.51%) ⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us.

@ghutchis
Copy link

I don't know, for example, if anything "in the wild" only prints the lattice parameters. For a while now, Avogadro writes the lattice vectors, e.g.

KBr.txt

Happy to continue the discussion. I'm building up the interactive optimization and several people (including myself) would like to use xtb methods. Being able to write cjson instead of Turbomole coords would be nice, although I implemented that recently.

On the writer side, there's a lot that can be supported or improved. Happy to continue the conversation - probably better on the chemicaljson repository.

- test round-tripping
- document implemented format
@awvwgk
Copy link
Member Author

awvwgk commented Jul 26, 2022

I think I got the format implemented now. Avogadro 2 happily reads the cjson files I'm producing.

@awvwgk awvwgk merged commit abfbff1 into grimme-lab:main Jul 26, 2022
@awvwgk awvwgk deleted the cjson branch July 26, 2022 19:11
@matterhorn103
Copy link

@awvwgk Could the support for cjson be mentioned in the docs? :)

@awvwgk
Copy link
Member Author

awvwgk commented Dec 4, 2023

Please open an issue in the respective repo in the future.

Updating the table in grimme-lab/xtb_docs#115

@matterhorn103
Copy link

Sure, sorry! Still getting used to the structure of open source projects e.g. that docs are typically in a separate repo

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