Skip to content

electron-check warnings #140

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.

Sign up for GitHub

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

Jump to bottom
Merged
YAY-C merged 4 commits into from
Jan 28, 2026
Merged

electron-check warnings #140

YAY-C merged 4 commits into from
Jan 28, 2026

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.

3 participants

@YAY-C @briling