Skip to content

electron-check warnings #140

Merged
YAY-C merged 4 commits intomasterfrom
warnings_handling
Jan 28, 2026
Merged

electron-check warnings #140
YAY-C merged 4 commits intomasterfrom
warnings_handling

Conversation

@YAY-C
Copy link
Contributor

@YAY-C YAY-C commented Jan 26, 2026 •
edited
Loading

  • add verbose kwarg to spahm call functions
  • pass verbose argument to check_nelec() function (for warning only)
  • add verbose arguments to functions calling check_nelec() for rho-SPAHM reps

@YAY-C YAY-C self-assigned this Jan 26, 2026
@YAY-C YAY-C added enhancement New feature or request in progress labels Jan 26, 2026
@YAY-C
Copy link
Contributor Author

YAY-C commented Jan 26, 2026

discussion from #139 :
Ksenia:

the second commit belongs to a different PR i think...

but importantly, it changes the behavior, because the check_nelec() function also raises an error if there is too many electrons to fit into existing basis. we added it after fixing the minao-without-ecp bug for Iodine, and it should catch similar bugs for other bases. so the options are:

  1. rename the argument to ignore_too_many_electrons/allow_too_many_electrons or something (so it looks dangerous) but i think it's still dangerous
  2. pass this verbose argument to check_nelec() so the error is raised but the warning is suppressed
  3. option 2) but rename it to suppress_full_shell_warning or something (i think verbose is too vague)

Liam:

I'm partial to a modified option 3 ("suppress_warnings", in case other warnings get added in the future)

@YAY-C
Copy link
Contributor Author

YAY-C commented Jan 26, 2026

@briling

why do you have full-shell molecules anyway?

Working with L11 database for the COSMO cookbook-recipe (as a quick example of KRR using e-spahm).

@briling
Copy link
Contributor

briling commented Jan 27, 2026

somehow you still have the get_cos() fix here so rebase!

@YAY-C YAY-C force-pushed the warnings_handling branch from efe9644 to ae80d2d Compare January 27, 2026 20:05
@YAY-C YAY-C requested a review from briling January 28, 2026 08:52
YAY-C
YAY-C commented Jan 28, 2026
View reviewed changes
qstack/spahm/rho/utils.py
if readdm is None:
_e, v = spahm.get_guess_orbitals(mol, guess, xc=xc)
dm = guesses.get_dm(v, mol.nelec, mol.spin if sp is not None else None)
dm = guesses.get_dm(v, mol.nelec, mol.spin if sp is not None else None, verbose=printlevel+1)
Copy link
Contributor Author

@YAY-C YAY-C Jan 28, 2026

Choose a reason for hiding this comment

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

@briling I'm not a big fan of the increment startegy here, but seems to be the best option...
what do you think?

Copy link
Contributor

@briling briling Jan 28, 2026

Choose a reason for hiding this comment

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

i think it's okay!

briling
briling approved these changes Jan 28, 2026
View reviewed changes
Copy link
Contributor

@briling briling left a comment

Choose a reason for hiding this comment

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

can you squash and merge?

qstack/spahm/rho/utils.py
if readdm is None:
_e, v = spahm.get_guess_orbitals(mol, guess, xc=xc)
dm = guesses.get_dm(v, mol.nelec, mol.spin if sp is not None else None)
dm = guesses.get_dm(v, mol.nelec, mol.spin if sp is not None else None, verbose=printlevel+1)
Copy link
Contributor

@briling briling Jan 28, 2026

Choose a reason for hiding this comment

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

i think it's okay!

@YAY-C YAY-C merged commit 9b9a092 into master Jan 28, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@briling briling briling approved these changes

Assignees

@YAY-C YAY-C

Labels

enhancement New feature or request in progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@YAY-C @briling