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

Consider replacing CrystalInfoFramework.jl with cif_api_jll #31

Open
kbarros opened this issue Aug 23, 2022 · 3 comments
Open

Consider replacing CrystalInfoFramework.jl with cif_api_jll #31

kbarros opened this issue Aug 23, 2022 · 3 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@kbarros
Copy link
Member

kbarros commented Aug 23, 2022

CrystalInfoFramework.jl is a fairly heavy dependency, and accounts for almost half of the load time for using Sunny, even if the user never reads any .cif files.

Should we move to the C library cif-api?
https://comcifs.github.io/cif_api/index.html

It is already distributed by Julia package manager as cif_api_jll . Example code is here:
https://github.com/jamesrhester/CrystalInfoFramework.jl/blob/master/src/libcifapi.jl

Current blocker: It seems that cif-api is not currently working on Windows? For details, see: https://github.com/search?q=repo%3Ajamesrhester%2FCrystalInfoFramework.jl%20windows&type=code

@kbarros
Copy link
Member Author

kbarros commented Sep 22, 2022

Progress in Julia itself may reduce load time issues https://sciml.ai/news/2022/09/21/compile_time/ , so I think this becomes very low priority.

@kbarros
Copy link
Member Author

kbarros commented Sep 25, 2022

There is some renewed motivation for this, because cif-api may be more robust to errors in the CIF files. Xiaojian found a CIF file which causes Sunny to error with Lerche.UnexpectedToken (probably a parsing error).
Bi3FeO4Mo2O8_setting2.cif.zip

@kbarros
Copy link
Member Author

kbarros commented Aug 27, 2023

Regarding timing, situation seems to be improved in Julia 1.10. On my machine using Sunny is 1.3s, while using CrystalInfoFramework is 0.6s. However, most of the dependencies of CrystalInfoFramework would be required by Sunny anyway, which can be inspected using:

@time_imports using CrystalInfoFramework

Perhaps we wouldn't need DataFrames and Lerche, which combined account for about 0.23s of loading time on my machine.

@kbarros kbarros added the enhancement New feature or request label Nov 16, 2024
@kbarros kbarros added the help wanted Extra attention is needed label Dec 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant