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

Removed uneccesary minus sign in ROHF stability analysis. #1142

Merged
merged 1 commit into from Aug 20, 2018

Conversation

mdav2
Copy link

@mdav2 mdav2 commented Aug 17, 2018

Description

Changes L1291 in libscf_solver/rohf.cc to read:
outfile->Printf(" Lowest ROHF->ROHF stability eigenvalues:\n");
Rather than:
outfile->Printf(" Lowest ROHF->ROHF stability eigenvalues:-\n");

The minus sign seems ambiguous.

Status

  • Ready for review
  • Ready for merge

@loriab
Copy link
Member

loriab commented Aug 17, 2018

It's been there since forever. But yeah, I don't see why. Any insight, @andysim ?

Copy link
Member

@dgasmith dgasmith left a comment

Choose a reason for hiding this comment

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

LGTM. Can you remove the parts of your issue template that are not relevant?

@andysim
Copy link
Member

andysim commented Aug 20, 2018

@loriab it's how English schools used to teach kids to introduce lists. Shouldn't be used anymore, but I didn't know that at the time of writing that code, apparently. It's also considered perfectly fine to use 'which' to introduce a restricted relative clause in British English so that appears in some of my writing circa 2004; it's more than just s/s/z/ and s/ou/o/ that's needed to translate between the two!

Copy link
Member

@andysim andysim left a comment

Choose a reason for hiding this comment

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

LGTM

@andysim andysim merged commit d0620d9 into psi4:master Aug 20, 2018
@loriab loriab added this to the Psi4 1.3 milestone Aug 24, 2018
@dgasmith dgasmith mentioned this pull request Feb 5, 2019
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.

None yet

4 participants