Skip to content

Refactor the crosslink class and add tests to better generate simple randomwalkpaths using iterative methods#1314

Open
CalCraven wants to merge 12 commits intomosdef-hub:developfrom
CalCraven:develop
Open

Refactor the crosslink class and add tests to better generate simple randomwalkpaths using iterative methods#1314
CalCraven wants to merge 12 commits intomosdef-hub:developfrom
CalCraven:develop

Conversation

@CalCraven
Copy link
Copy Markdown
Contributor

No description provided.

elif radius and spacing:
N = int((2 * np.pi * radius) / spacing)
else:
spacing = (2 * np.pi * radius) / N

Check notice

Code scanning / CodeQL

Unused local variable Note

Variable spacing is not used.

logger = logging.getLogger(__name__)

def get_second_point(state, existing_points, check_path, first_point):

Check notice

Code scanning / CodeQL

Explicit returns mixed with implicit (fall through) returns Note

Mixing implicit and explicit returns may indicate an error, as implicit returns always return None.
Copy link
Copy Markdown
Contributor

@chrisjonesBSU chrisjonesBSU left a comment

Choose a reason for hiding this comment

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

Working my way through this. Here are some things I noticed so far.

pos1[:, 1] = np.arange(10)
path.append_coordinates(pos1)
path.form_linear_bond_graph()

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable Error test

Local variable 'p_value' may be used before it is initialized.
Comment on lines +463 to +470
assert path.bond_graph.number_of_nodes() == 10

def test_init_positions(self):

# Random point
path = hard_sphere_random_walk(
radius=1.0,
bond_length=1.5,

Check failure

Code scanning / CodeQL

Wrong name for an argument in a call Error test

Keyword argument 'min_angle' is not a supported parameter name of
function hard_sphere_random_walk
.
Keyword argument 'max_angle' is not a supported parameter name of
function hard_sphere_random_walk
.
tolerance=1e-5,
trial_batch_size=20,
chunk_size=512,
run_on_gpu=False,

Check failure

Code scanning / CodeQL

Potentially uninitialized local variable Error

Local variable 'kwargs' may be used before it is initialized.

Check warning

Code scanning / CodeQL

Redundant comparison Warning test

Test is always true, because of
this condition
.
)
is_inside_mask = state.volume_constraint.is_inside(
points=xyzs, buffer=state.radius
)

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'is_inside_mask' is unnecessary as it is
redefined
before this value is used.
state.gpu_static_points = cuda.to_device(static_points)

# Main random walk loop
walk_finished = False

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'existing_points' is unnecessary as it is
redefined
before this value is used.

# Prepare GPU static points if using GPU
if state.run_on_gpu:
from numba import cuda

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'is_inside_mask' is unnecessary as it is
redefined
before this value is used.
The Path object to populate with coordinates
N : int, required
Number of sites in the path
spacing : float, default = 1.0 nm

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'rot_matrix' is unnecessary as it is
redefined
before this value is used.

def visualize(self, radius=0.1):
"""Visualize in 3D space using py3Dmol of the Path as a Compound.

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning

This assignment to 'view' is unnecessary as it is
redefined
before this value is used.
Comment on lines +204 to +206
indices[0],
name=self.beads[indices[0]],
xyz=self.coordinates[indices[0]],

Check warning

Code scanning / CodeQL

Unreachable code Warning

This statement is unreachable.
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