Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

PyCall → PythonCall #38

Merged
merged 53 commits into from
Aug 14, 2024
Merged

PyCall → PythonCall #38

merged 53 commits into from
Aug 14, 2024

Conversation

Alexsp32
Copy link
Member

@Alexsp32 Alexsp32 commented Jul 22, 2024

Update:

I've decided to move this to a separate package instead to avoid requiring Python in NQCModels.jl.

Instead, this PR now makes the ASE and ASE friction interfaces usable with PythonCall.jl to fit in the structure of using PythonCall and CondaPkg.jl for packaging Python dependencies.


Avoid ase calculator and as many Julia -> Python -> Torch pipelines as reasonably possible without re-writing MACE for Julia.

@Alexsp32
Copy link
Member Author

Alexsp32 commented Aug 1, 2024

I've decided to move this to a separate package instead to avoid requiring Python in NQCModels.jl.

Instead, this PR now makes the ASE and ASE friction interfaces usable with PythonCall.jl to fit in the structure of using PythonCall and CondaPkg.jl for packaging Python dependencies.

@Alexsp32 Alexsp32 changed the title DRAFT: MACE interface DRAFT: PyCall → PythonCall Aug 1, 2024
@Alexsp32 Alexsp32 marked this pull request as ready for review August 2, 2024 13:08
@Alexsp32 Alexsp32 changed the title DRAFT: PyCall → PythonCall PyCall → PythonCall Aug 13, 2024
@Alexsp32 Alexsp32 requested a review from reinimaurer1 August 13, 2024 11:48
@@ -42,7 +42,12 @@ jobs:
run: julia -e 'using Pkg; Pkg.Registry.add(RegistrySpec(url="https://github.com/JuliaMolSim/MolSim"))'

- uses: julia-actions/julia-buildpkg@v1
env:
LD_LIBRARY_PATH: $HOME/libxc-5.1.5/lib
Copy link
Member

Choose a reason for hiding this comment

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

Why is there a libxc dependency here? this only seems to be in the CI. does anything in NQCModels or its tests require that?

Copy link
Member Author

Choose a reason for hiding this comment

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

test/ase_pycall.jl:17-22 uses it to test the ASE calculator interface with GPAW. I couldn't find a working conda package for GPAW, or Id've changed the test mechanism to install it through CondaPkg.jl and make the tests work on any machine without having to set up ase and GPAW first.


@testset "EMT" begin
emt = pyimport("ase.calculators.emt")
h2.calc = emt.EMT()
Copy link
Member

Choose a reason for hiding this comment

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

just a note: it used to be that calculators require setting with atoms.set_calculator(calc). EMT might be a rare exception where you can directly set it like this

Copy link
Member

@reinimaurer1 reinimaurer1 left a comment

Choose a reason for hiding this comment

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

all great. I only left some comments

@Alexsp32 Alexsp32 merged commit ed5d2cc into main Aug 14, 2024
1 check passed
@Alexsp32 Alexsp32 deleted the mace-calculator branch August 14, 2024 08:10
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants