Skip to content

Assign device for calculating minimum distances as GPU, if available. - #43

Closed
messerlyra wants to merge 1 commit into
lanl:mainfrom
messerlyra:calc_min_dist_device
Closed

messerlyra wants to merge 1 commit into
lanl:mainfrom
messerlyra:calc_min_dist_device

Conversation

@messerlyra

Copy link
Copy Markdown
Contributor

Calculate_min_dists was defaulting to CPU. Now explicitly assigned to GPU.

Also, removing duplicate call of calculate_min_dists

@messerlyra messerlyra left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure how, but hippynn_interface.py had a duplicate call of calculate_min_dists. The first call still used the old format where kwargs were not named explicitly. Removed that call and included the device specification in the second call.

@bnebgen-LANL

Copy link
Copy Markdown
Collaborator

You must trim the database (database.arr_dict[curK] = database.arr_dict[curK][min_dists_flag]) before setting natoms, otherwise the natoms array will be of dimention miss-match with the remainder of the database.

bnebgen-LANL pushed a commit that referenced this pull request Oct 28, 2026
In ASE, empty cells made with a defined cell default to pbc=False. In
simple_condensed_phase_builder_task, this has been  updated to
explicitly enforce pbcs.

This also includes edits from pull request #43. The second database
cleaning has been removed from hippynn_inerface and the gpu device has
been added for neighbor finding.
@bnebgen-LANL

Copy link
Copy Markdown
Collaborator

Edits fixed and included in #44

@messerlyra

messerlyra commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor Author

@bnebgen-LANL OK, now I get what your comment means. You were answering my question about why we call calc_min_dists twice. Basically, we need to trim the array based on distances before calculating n_atoms. That makes sense to have calc_min_dists before n_atoms. It appears that PR #44 removes the second distance trimming, but it still calls calc_min_dists a second time, which seems unnecessary since min_dists is never used again.

Also, the other key part of this PR was to set device=device_cuda in calc_min_dists. But it appears this gets resolved in PR #44 . Is that correct?

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.

2 participants