Skip to content

Conversation

@fcuantico
Copy link
Contributor

Adding the lenard-jones potential

Is this pull request associated with an issue(s)?
Please list issues associated with this pull request, including closing words
as appropriate.

Description
Describe what this pull request will accomplish.

TODOs
For draft pull requests please include a list of what needs to be done and check
off items as you complete them.

Adding the lenard-jones potential
@fcuantico fcuantico changed the base branch from pyberny_addition to master February 11, 2025 22:56
@fcuantico fcuantico changed the base branch from master to pyberny_addition February 11, 2025 22:58
Base automatically changed from pyberny_addition to master February 14, 2025 17:12
Copy link
Member

@ryanmrichard ryanmrichard left a comment

Choose a reason for hiding this comment

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

This looks like it's basically r2g.

Copy link
Member

@jwaldrop107 jwaldrop107 left a comment

Choose a reason for hiding this comment

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

I think the tests are passing as well as they're going to for now. I don't think the clang failure is resolvable at this time. We can probably go ahead and merge this.

@ryanmrichard
Copy link
Member

@jwaldrop107 the error:

1:     log('Energy: {:.12}'.format(energy))
1: TypeError: unsupported format string passed to tensorwrapper.Tensor.__format__

requires casting the tensor to a numpy array and printing the element. Similarly,

1: AssertionError: array(<tensorwrapper.Tensor object at 0x7f391e726930>, dtype=object) != -1.0

should be fixable by casting the object to a numpy array and then comparing the element to -1.0.

So I'd advocate for fixing the tests before merging.

@jwaldrop107
Copy link
Member

jwaldrop107 commented Mar 17, 2025

@ryanmrichard The first error is the same error that @jlheflin's change (which is to cast the offending tensorwrapper.Tensors into NumPy arrays) fixed in the GCC build, but doesn't fix for the Clang build. In the second error, the Tensor is already being cast as a NumPy array. I'm pretty sure these are both versions of the same issue that we bypassed in NWChemEx/FriendZone#21, which seems to stem from the difference in compilers used to compile the Python executable and our libraries.

@ryanmrichard
Copy link
Member

@jwaldrop107 If my suggestion fixes the error, then that should also fix FriendZone. I'm not sure what's going on with the logging issue, but PyBerny should not be seeing a tensorwrapper tensor.

@jwaldrop107
Copy link
Member

@ryanmrichard, NWChemEx/TensorWrapper#207 has this passing now.

@ryanmrichard ryanmrichard marked this pull request as ready for review March 25, 2025 21:32
@ryanmrichard ryanmrichard merged commit 9fe4760 into master Mar 25, 2025
8 of 9 checks passed
@ryanmrichard ryanmrichard deleted the fcuantico-patch-1 branch March 25, 2025 21:32
@jwaldrop107
Copy link
Member

🚀 [bumpr] Bumped! New version:v0.0.2 Changes:v0.0.1...v0.0.2

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.

5 participants