Skip to content

Conversation

@aleaverfay
Copy link
Collaborator

FA-elec refactored for rotamer pair / sparse block pair evaluation.

Unit tests for sparse evaluation now properly working.

Torch 2.5.0, 2.5.1, and 2.7 also seem to work just fine for tmol; Torch 2.4 does not work.

Also updating the code relying on cattrs.structure

The newest versions of cattrs (>=24.1.0) throw errors that previously were
not errors or that could be worked around or something so that now
the code in tmol/chemical/restypes.py produces errors that weren't
there before.

One small issue: numpy.array cannot be used as a type name for attrs
and this produces a weird complaint out of cattrs. numpy.array is
for creating ndrarrays; ndarrays are the types, array is the function
Nicely enough, termini variants are being properly handled
already; the tests simply had to be updated.
Now the sampler is responsible for mapping their samples onto the
DOFs that define the conformations, allowing samplers to define more
than only the chi DOFs for the samples; e.g., if I want to build
hydroxyl rotamers onto the heavy-atom coordinates of an existing SER
This code is "used" by the on-the-fly RPE calculation code
which itself is not really used
This allows me to talk about global-block-type index (not a great name)
when building rotamers for disallowed block types as will happen
when a block has all of its block-types disallowed.
Pack rotamers is now agreeing with the final energy after scoring
This includes a bug in neighbor detection that was allowing the
first rotamer from pose i+1 to get checked as a neighbor with
rotamers of the last residue of pose i!
* Update modern GPU to CUDA13 compatibility (See moderngpu/moderngpu#56)
* Add conda/mamba-installed nvtx to search path
* Update NVidia conda channel to  torch 2.8.0 for CUDA 13 compatibility
* Add nvtx to conda packages
* Fix broken unit test enable_tensor_view that lead to a "Illegal Operation" error
* Change error-propogation command location in "linting" script until after conda activation
@aleaverfay aleaverfay requested a review from jflat06 October 6, 2025 18:43
@jflat06
Copy link
Collaborator

jflat06 commented Oct 6, 2025

Have you had a chance to benchmark this vs the old code?

Copy link
Collaborator

@jflat06 jflat06 left a comment

Choose a reason for hiding this comment

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

Seems like there's stuff from the packing branch and master mixed in with this, so I did not review that.

The elec code seems fine I think. I had some comments on the testing code.

// reindexing function
nvtx_range_push("dispatch::segscan");
auto k_reindex = [=] MGPU_DEVICE(int index, int seg, int rank) {
if (nodestart + index >= nodes.size(0) || nodestart + index < 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this and the other unrelated stuff is just coming from master?

Comment on lines +440 to +454
assert(first_rot_block_type.size(0) == n_poses);
assert(first_rot_block_type.size(1) == max_n_blocks);

assert(block_ind_for_rot.size(0) == n_rots);
assert(pose_ind_for_rot.size(0) == n_rots);
assert(block_type_ind_for_rot.size(0) == n_rots);

assert(n_rots_for_pose.size(0) == n_poses);
assert(rot_offset_for_pose.size(0) == n_poses);

assert(n_rots_for_block.size(0) == n_poses);
assert(n_rots_for_block.size(1) == max_n_blocks);

assert(rot_offset_for_block.size(0) == n_poses);
assert(rot_offset_for_block.size(1) == max_n_blocks);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we've established a common set of parameters, I was thinking it might be good to make a function for sanity checks like this to cover checking the common parameters.

Just an idea, not something for this PR.

Comment on lines 573 to 574
if (block_type1 < 0 || block_type2 < 0) {
return;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We decided we didn't need this, right?

Comment on lines +229 to +240
@classmethod
def get_block_pair_dispatch_indices(cls, nres, device):
i_p = []
i_r1 = []
i_r2 = []
for i in range(nres):
for j in range(nres):
if i <= j:
i_p += [0]
i_r1 += [i]
i_r2 += [j]
return torch.tensor([i_p, i_r1, i_r2], dtype=torch.int32, device=device)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be unused and deleted, not sure why it's added back.

Comment on lines +208 to +227
def get_whole_pose_scorer(cls, pose_stack, param_db, device):
energy_term = cls.energy_term_class(param_db=param_db, device=device)

for bt in pose_stack.packed_block_types.active_block_types:
energy_term.setup_block_type(bt)
energy_term.setup_packed_block_types(pose_stack.packed_block_types)
energy_term.setup_poses(pose_stack)

return (
energy_term.render_whole_pose_scoring_module(pose_stack)
if block_pair_scoring is False
else energy_term.render_block_pair_scoring_module(pose_stack)
)
return energy_term.render_whole_pose_scoring_module(pose_stack)

@classmethod
def get_block_pair_scorer(cls, pose_stack, param_db, device):
energy_term = cls.energy_term_class(param_db=param_db, device=device)

for bt in pose_stack.packed_block_types.active_block_types:
energy_term.setup_block_type(bt)
energy_term.setup_packed_block_types(pose_stack.packed_block_types)
energy_term.setup_poses(pose_stack)

return energy_term.render_block_pair_scoring_module(pose_stack)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want 2 completely separate functions for this? Seems like a bit of a waste to duplicate the code for essentially a 1 line difference.

Comment on lines +430 to +444
# reshape these values for the new block-pair scoring scheme
gold_vals_upper_triangle = numpy.zeros_like(gold_vals)
assert gold_vals.shape[2] == gold_vals.shape[3]
n_terms = gold_vals.shape[0]
n_poses = gold_vals.shape[1]
n_res = gold_vals.shape[2]
assert n_poses == 1
inds_arange = numpy.arange(n_res, dtype=int)
inds_arange_i = inds_arange.reshape(-1, 1)
inds_arange_j = inds_arange.reshape(1, -1)
ij_is_upper_triangle = inds_arange_i < inds_arange_j
ij_is_lower_triangle = inds_arange_i > inds_arange_j
ij_is_diagonal = inds_arange_i == inds_arange_j
gold_vals_upper_triangle[:, :, ij_is_diagonal] = gold_vals[:, :, ij_is_diagonal]
gold_vals_upper_triangle[:, :, ij_is_upper_triangle] = 2 * gold_vals[:, :, ij_is_upper_triangle]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mind elaborating a bit on this code? I'm not sure I follow on why this is necessary.

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.

3 participants