Conversation
Pull Request Test Coverage Report for Build 5047790300
💛 - Coveralls |
|
@niklassiemer I didn't get to working on our conda problem, but @raynol-dsouza's PR got an even more informative error message than either of us!!! It's late on a Friday for me so I've got to stop working, but I'm super optimistic that we now have enough data to resolve this. It's very weird that our three PRs terminally crashed at such different stages of this error though... if we'd just gotten this error message to start with it would have been much more clear how to proceed.... |
So on the scipy requirements, I see the lower pin to Ok, that's just because the git head is not 0.20.0 anymore. Indeed, on 0.20.0 the requirement is there. I'm going to check if 0.21.0 (which only has the lower bound) is available on conda yet... Nope. Not showing on conda releases and there's no PR for it yet on their feedstock. However, as soon as it's released, bumping our dependency to 0.21.0 should resolve all the currently failing tests. |
|
@liamhuber thank you for getting this sorted! Shall I go ahead and merge this PR? |
liamhuber
left a comment
There was a problem hiding this comment.
@raynol-dsouza, lgtm. I have no idea how these jobs actually run or what the rest of their code looks like, but I trust you that it's performing as expected.
I made a few minor comments for (hopefully) improving readability.
Co-authored-by: Liam Huber <liamhuber@greyhavensolutions.com>
| for l in snap_lines[1:]: | ||
| temp_list.append([float(n) for n in l.split()[1:]]) # first column is the species. Ignore it. | ||
| trajectory.append(temp_list) | ||
| start = stop |
There was a problem hiding this comment.
I still find this formulation really difficult to read. Could you switch it with start = starts[i-1] and move it up to the top?
There was a problem hiding this comment.
starts[i-1] will become starts[-1] when i=0. I kept it this way to avoid this. Looking at it again, I think start=starts[i] at the beginning of the loop should work.
There was a problem hiding this comment.
Looking at it again, I think start=starts[i] at the beginning of the loop should work.
Good catch! Yeah, that should be perfect
liamhuber
left a comment
There was a problem hiding this comment.
I still have one nit about start = stop, but otherwise looks better!
PiMD jobs now only collect the beads' centroid positions and forces instead of the positions and forces of all the beads.