Skip to content

Trap PairList errors#793

Merged
drroe merged 6 commits into
Amber-MD:masterfrom
drroe:improveCheck
Jan 28, 2020
Merged

Trap PairList errors#793
drroe merged 6 commits into
Amber-MD:masterfrom
drroe:improveCheck

Conversation

@drroe

@drroe drroe commented Jan 23, 2020

Copy link
Copy Markdown
Contributor

Errors that occur when setting up a PairList (due to e.g. corrupt coordinates) are now trapped. The total number of atoms off the grid are now reported instead of a message for every atom off the grid. Atoms that end up off the grid are now counted as problems by the check action. Pair list errors in energy calculations are now properly handled.

Daniel R. Roe added 5 commits January 23, 2020 08:48
…icates something really wrong with the input coords or box and shouldnt be glossed over. Change CalcEnergy to CalcNonbondEnergy to better reflect what is actually going on.
…1 warning indicating how many atoms are off the grid.
… Check will consider off-grid atoms as problems.
@drroe drroe self-assigned this Jan 23, 2020
@drroe

drroe commented Jan 23, 2020

Copy link
Copy Markdown
Contributor Author

It might be just about time to drop appveyor support. Failure is build system related and I'm tired of stamping out the unnecessary brushfires...

@drroe

drroe commented Jan 23, 2020

Copy link
Copy Markdown
Contributor Author

@hainm

hainm commented Jan 23, 2020

Copy link
Copy Markdown
Contributor

I really don't know why. May be because of using python 2.7.
I have disabled testing of test_actionlist.py (that might cause the hang). Please restart the build. thanks.

@swails

swails commented Jan 23, 2020

Copy link
Copy Markdown
Contributor

We stopped using python 2, I’m pretty sure. And that’s not a cause. “Removing an infinite hang” wasn’t one of the breaking changes they made in the migration to Python 3 ;).

@hainm

hainm commented Jan 23, 2020

Copy link
Copy Markdown
Contributor

We stopped using python 2, I’m pretty sure.

What do you mean? #790

And that’s not a cause.

may be there's a memory leak in pytraj :D

@drroe

drroe commented Jan 24, 2020

Copy link
Copy Markdown
Contributor Author

Any idea why we've got this pytraj hang? It shouldn't be related to these changes. I'd like to merge this but if there are any concerns I won't.

@swails

swails commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

pytraj has been hanging for awhile. I suspect it’s some deadlock in one of the parallel tests. It’s possible the problem lies in the mixed parallelism of the test framework with the tests themselves. The Jenkins nodes report having many CPUs while the Travis executors will report 1. I suspect the test framework will parallelize based on the number of available cpus, which would cause Jenkins to regularly hang while Travis wouldn’t.

“Python 2” and “memory leak” wouldn’t cause a deadlock. I’d start by disabling the pytraj parallel tests. But I’ll defer to Hai on how to do that.

@hainm

hainm commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

I suspect it’s some deadlock in one of the parallel tests.

seems reasonable to me too.

I’d start by disabling the pytraj parallel tests. But I’ll defer to Hai on how to do that.

I actually disabled the multiprocessing tests in Jenkins originally but forgot to turn off the MPI tests as well. I have made change here: https://github.com/Amber-MD/pytraj/blob/8acf4ab1e8538d8f9b3c71f7539e2229a3028a40/Jenkinsfile#L33

@hainm

hainm commented Jan 24, 2020

Copy link
Copy Markdown
Contributor

run jenkins

@drroe

drroe commented Jan 28, 2020

Copy link
Copy Markdown
Contributor Author

@hainm Since pytraj compiles and tests fine with this branch on my local machine, I'm going to chalk the Jenkins failure up to something strange with the build machinery and merge this.

@drroe drroe merged commit b836078 into Amber-MD:master Jan 28, 2020
@drroe drroe deleted the improveCheck branch January 28, 2020 19:28
@hainm

hainm commented Jan 28, 2020

Copy link
Copy Markdown
Contributor

yay.

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