Skip to content

Add Linear Nk OCCRI method + ISDF #155

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

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open

Conversation

kosm6966
Copy link

@kosm6966 kosm6966 commented Aug 1, 2025

##Summary

• Add OCCRI method for efficient exact exchange evaluation in periodic systems with significant algorithmic improvements.
• Performance is similar to but typically faster than existing occri method
• Designed to integrate with isdfx
• Provide memory-efficient implementations with MO blocking following PySCF patterns
• Implement ISDFX (Interpolative Separable Density Fitting eXchange) module with comprehensive testing and examples
• Exchange build is linear with N_k as shown by Example 2
• Default threshold is set and provides < 50 μHa error for all test systems
• Include comprehensive test suites and educational examples demonstrating usage and performance

Features

• OCCRI: Exchange matrix evaluation using occupied orbital resolution of identity
• ISDFX: Tensor hypercontraction-based exchange with interpolative separable density fitting
• Support for RHF, UHF, RKS, UKS and k-point variants (KRHF, KUHF, KUKS, KRKS)
• Memory management with automatic MO blocking for large systems
• Comprehensive utility functions separated from object methods for better code organization
• Natural orbital construction and exchange matrix building utilities

Performance & Accuracy

• ISDFX provides near-FFTDF accuracy (typically <50 μHa energy differences)
• Scaling analysis shows sub-quadratic performance with k-point sampling
• Memory-efficient MO blocking prevents memory overflow for large systems
• Proper handling of asymmetric k-point meshes and complex conjugation

Testing & Examples

• 84 comprehensive tests covering energy accuracy, k-point handling, and edge cases
• Energy comparison tests with 50 μHa tolerance vs FFTDF reference
• Asymmetric k-point tests to catch symmetry and conjugation bugs
• Educational examples: basic usage, AFM initial guess setup, scaling analysis
• Integration with existing run_tests.py framework with --perf flag support

Code Quality

• Follows PySCF coding conventions with comprehensive docstrings
• Separation of pure algorithms from object methods for better maintainability
• Formatted with Black, isort, and flake8
• Extensive error handling and input validation
• Memory cleanup following PySCF patterns

Test plan

  • All 84 tests pass including energy accuracy and performance tests
  • Examples demonstrate proper usage, AFM setup, and scaling behavior
  • Code quality checks pass (Black, isort, flake8)
  • Memory management validated with MO blocking implementation
  • K-point handling thoroughly tested with asymmetric meshes

kosm6966 added 27 commits July 18, 2025 12:28
@kosm6966 kosm6966 changed the title Add OCCRI (Occupied Orbital Coulomb Resolution of Identity) method Add Linear Nk OCCRI method + ISDF Aug 15, 2025
@jeanwsr
Copy link
Contributor

jeanwsr commented Aug 17, 2025

Is it better to put these codes in pbc/df?

const int nthreads = omp_get_max_threads();

// Initialize FFTW multithreading support
if (!fftw_init_threads()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There are several exit() in this c code. Could you return these errors to the Python side and raise an exception in Python?


def __init__(
self,
mydf,
Copy link
Contributor

Choose a reason for hiding this comment

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

  • This requires a PBC mean-field instance. mydf often refers to the density fitting object. It's better to use a different name.

  • The OCCRI object does not depend on the mean-field instance. Passing it to the __init__ function looks a little bit misleading. I think it's better to provide a factory function or a class method like OCCRI.from_mf(mf), which create a OCCRI instance and validate the mfthere.

if kpts is None:
kpts = numpy.zeros(3, numpy.float64)
kpts = kpts.round(6)
self.kmesh = [
Copy link
Contributor

Choose a reason for hiding this comment

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

This can potentially result in incorrect kmesh for numerical issues, or non-orthogonal lattice. It's recommended to convert kpts to kmesh using this API https://github.com/pyscf/pyscf/blob/fafef00a03b28a9ddbe23cad2d99ce0f1222180c/pyscf/pbc/tools/k2gamma.py#L39


# Step 1: Get pivot points
logger.debug(self, "Selecting ISDFX pivot points")
interpolation.get_pivots(self)
Copy link
Contributor

Choose a reason for hiding this comment

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

The pivots, aovals, and W attributes of self are initialized in the interpolation module. It's recommend to return these variables and explicitly assign them to in this build method.


return make_natural_orbitals(self.cell, self.kpts, dms)

def __del__(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you create an empty __del__?

@@ -0,0 +1,20 @@
# Copyright 2025 The PySCF Developers. All Rights Reserved.
Copy link
Contributor

Choose a reason for hiding this comment

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

The __init__.py in the test directory will cause the test code to be included in the wheel package. This is typically not needed and can be removed.

@@ -0,0 +1,167 @@
#!/usr/bin/env python
Copy link
Contributor

Choose a reason for hiding this comment

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

The pytest suite is already configured in the package. It can automatically discover all tests. A manual test driver is unnecessary.

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