Skip to content

Conversation

@ohzde
Copy link
Contributor

@ohzde ohzde commented Jul 14, 2025

This PR adds a screening feature to Gbasis for cases involving one contractions. It adds screening for all evaluations (eval.py, eval_deriv.py, density.py, and stress_tensor.py) via the tol_screen parameter, and the screening can be turned on or off with the screen_basis parameter. A new function, evaluate_contraction_mask(), has been added to the screening module for this purpose.

Checklist

  • Write a good description of what the PR does.
  • Add tests for each unit of code added (e.g. function, class)
  • Update documentation
  • Squash commits that can be grouped together
  • Rebase onto master

Type of Changes

Type
🐛 Bug fix
✨ New feature
🔨 Refactoring
📜 Docs

Related

to resolve #121, related to #191

@ohzde
Copy link
Contributor Author

ohzde commented Jul 14, 2025

hi, the following is a notebook testing all of the evaluation screenings with tol_screen = 1e-8. it tests the screening on two systems: one is a peptapeptide, and the other is the system mentioned in #121. please let me know if I need to change anything. currently, there is no test targeting the effect of tol_screen on the error. If everything else looks good, please let me know so I can add the screening precision test.
one_index.pdf

@marco-2023
Copy link
Contributor

Hi @ohzde, I went through the branch code:

  • Fixed the bug we had in the screening function. We were selecting the screening radius based on the primitive with the higher exponent. Sometimes, in some contractions (In some basis, there are several contractions per shell), there were different coefficients for the same primitive (one per contraction). Then, in some cases, the wrong coefficient was used for screening, giving incorrect results. Now the screening R is computed for every primitive in every contraction, and the biggest one is used.
  • Removed the screening for the derivatives of the basis (the screening function is different for derivatives).
  • Removed the basis evaluation screening from normal tests.

I will be merging this.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds screening functionality to Gbasis for one-index evaluations to improve computational efficiency. It introduces screening based on distance cutoffs between contraction centers and evaluation points, with configurable tolerance parameters.

Key changes:

  • Added screening functions to determine when basis function evaluations can be skipped based on distance cutoffs
  • Updated evaluation functions to support optional screening with screen_basis and tol_screen parameters
  • Modified existing tests to explicitly disable screening to maintain compatibility with reference values

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
gbasis/screening.py Added new screening functions including primitive cutoff radius computation and point masking
gbasis/evals/eval.py Modified basis evaluation to support optional screening with distance-based cutoffs
gbasis/evals/density.py Added screening parameters to density evaluation function
tests/test_screening.py New comprehensive tests for screening functionality with various bond lengths and tolerances
tests/test_eval.py Updated existing tests to explicitly disable screening for reference comparisons
tests/test_density.py Updated density tests to disable screening for reference value validation
tests/test_density_direct.py Updated direct density tests to disable screening for consistency

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@ohzde
Copy link
Contributor Author

ohzde commented Sep 20, 2025

Thanks @marco-2023 for these. Please let me know if anything is remaining, and if you want me to add the screening to the tutorial notebooks as well.

@marco-2023
Copy link
Contributor

Hi @ohzde, it would be nice to add an "example" notebook using screening. It would be awesome if you could start it on a different PR.

Copy link
Member

@PaulWAyers PaulWAyers left a comment

Choose a reason for hiding this comment

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

LGTM

@marco-2023 marco-2023 merged commit 8bbf73b into theochem:master Sep 26, 2025
9 checks passed
@ohzde ohzde mentioned this pull request Sep 29, 2025
5 tasks
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.

[BUG] Big memory usage for density evaluation

3 participants