Fix LNONCOLLINEAR match in Outcar parser

[DanielYang59]

Summary

• Fix LNONCOLLINEAR match in Outcar parser, to fix noncollinear attribute of Outcar class has wrong value for VASP calculations with LNONCOLLINEAR=.TRUE. #4031
• Add unit test
• Avoid hard coding number of spaces in re pattern

For a future PR

• [Reverted] Turn on terminate_on_match to early exit upon match, I wish I didn't misunderstand the usage, but early exit should speed up the search?

[DanielYang59]

Thanks @KylinGuo for reporting this and providing the test OUTCAR. Can I have your permission to include your OUTCAR as a test file for pymatgen?

[KylinGuo]

Thanks @KylinGuo for reporting this and providing the test OUTCAR. Can I have your permission to include your OUTCAR as a test file for pymatgen?

It's my pleasure! Yes, please.

[DanielYang59]

Thanks a lot. All credit goes to you :)

[janosh]

thanks a lot for the fix @DanielYang59! 👍 almost ready to go

[janosh]

i'm happy to merge despite the unclear implications of terminate_on_match=True given tests pass

[janosh]

these lines suggest that the matches dict can contain less keys if terminate_on_match=True and so you might get a different self.data after self.data.update(matches). but given all tests pass, this may not happen in practice

pymatgen/src/pymatgen/io/pwscf.py

Lines 623 to 630 in edcd465

	matches = regrep(
	self.filename,
	patterns,
	reverse=reverse,
	terminate_on_match=terminate_on_match,
	postprocess=postprocess,
	)
	self.data.update(matches)

[janosh]

any concerns @mkhorton?

[DanielYang59]

I think it might be safer for me to revert those changes in terminate_on_match until I'm 100% percent it wouldn't cause any breakage.

Currently there must have been some part of Outcar not covered by unit test, because the reverse reading in #4033 seems completely broken in Windows, but tests are still passing.

I would keep this into my TODO list, and have a closer look later.

[janosh]

thanks for the lightning-fast fix @DanielYang59! 👍

[DanielYang59]

No problem. Thanks for questioning the usage of terminate_on_match, might look closer later :)