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

adding benzene and methane to molecule factory to remove from_pyquante #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hatemhelal
Copy link
Member

Closes #8

@hatemhelal hatemhelal requested a review from FNTwin October 1, 2024 20:11
@hatemhelal hatemhelal self-assigned this Oct 1, 2024
def test_benzene(func, benchmark):
mol = from_pyquante("c6h6")
mol = molecule("benzene")
basis = basisset(mol, "def2-TZVPPD")
basis = jax.device_put(basis)
Copy link
Collaborator

@FNTwin FNTwin Dec 3, 2024

Choose a reason for hiding this comment

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

I would say for a benchmark that basis set is kinda expensive (most likely we want to also run the benchmark as a gh action). Is there a reason why we are using two different basis sets for the benchmarks?

Aso the eri_basis_sparse test for the benzene fails with a:
[libprotobuf ERROR external/com_google_protobuf/src/google/protobuf/message_lite.cc:449] xla.cpu.CompilationResultProto exceeded maximum protobuf size of 2GB:

Copy link
Collaborator

Choose a reason for hiding this comment

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

If a single basis sets can be used we can just modify the file like this to slightly improve clarity + adding the benzene molecule to the test_integrals parametrize

import jax
import pytest

from typing import Literal
from mess.basis import basisset
from mess.hamiltonian import Hamiltonian, minimise
from mess.zeropad_integrals import overlap_basis_zeropad
from mess.integrals import (
    eri_basis_sparse,
    kinetic_basis,
    nuclear_basis,
    overlap_basis,
)
from mess.structure import molecule
from conftest import is_mem_limited

BASIS_SET : Literal[str] = "6-31g"

@pytest.mark.parametrize("func", [overlap_basis, overlap_basis_zeropad, kinetic_basis])
def test_benzene(func, benchmark):
    mol = molecule("benzene")
    basis = basisset(mol, BASIS_SET)
    basis = jax.device_put(basis)

    def harness():
        return func(basis).block_until_ready()

    benchmark(harness)


@pytest.mark.parametrize("mol_name", ["h2", "water", "benzene"])
@pytest.mark.parametrize(
    "func", [overlap_basis, kinetic_basis, nuclear_basis, eri_basis_sparse]
)
def test_integrals(mol_name, func, benchmark):
    mol = molecule(mol_name)
    basis = basisset(mol, BASIS_SET)
    basis = jax.device_put(basis)

    def harness():
        return func(basis).block_until_ready()

    benchmark(harness)


@pytest.mark.parametrize("mol_name", ["h2", "water"])
@pytest.mark.skipif(is_mem_limited(), reason="Not enough host memory!")
def test_minimise_ks(benchmark, mol_name):
    # TODO: investigate test failure with cpu backend and float32
    from jax.experimental import enable_x64

    with enable_x64(True):
        mol = molecule(mol_name)
        basis = basisset(mol, BASIS_SET)
        H = Hamiltonian(basis)
        H = jax.device_put(H)

        def harness():
            E, C, _ = minimise(H)
            return E.block_until_ready(), C.block_until_ready()

        benchmark(harness)

Copy link
Collaborator

@FNTwin FNTwin left a comment

Choose a reason for hiding this comment

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

See comments but the code works so I'll approve and leave the decision to modify or not to you

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.

Replacement implementation for from_pyquante
2 participants