Start of aesthetic fixing (tests only)

[brookehus]

I think everyone should just commit to this instead of all separate PRs. I've indicated a few places with NEC TODO (don't worry @Dom1L, I may do DL TODO too).

In my first commit I'm just making better descriptions for the tests in test_geometry_statistics. @coarse-graining/developers, have a look at this first commit and see if you like how I'm doing it. My goals are twofold:

1. Make sure the initial one-line description is informative, especially with respect to what it's testing (shape vs. actual values, etc)
2. Make sure the calculations within the test are justified.

Note: be sure to keep running nosetests to make sure that you aren't (unintentionally) changing any of the logic or missing variable names!

[Dom1L]

I really like the new descriptions, makes it much more easier to follow.

Don't hesitate to drop some TODO's for me

Something that would also be important would be variable names especially for the angle etc. calculations with the double loops. But just as I'm writing, you already changed it :)

[brookehus]

Perhaps the thing to do here is focus on tests only for this PR. Then we can look at the real code documentation in a future PR.

@Dom1L you are responsible for the docs of any tests you wrote!

@nec4 you have your work cut out for you in test_utils, _divergence, and _nnet. Let me know if any are my responsibility with TODOs.

[nec4]

Sounds good!

[brookehus]

@nec4, added some comments to your tests and some changes (commit).

In general a few things:

1. Use spacing to help organize
2. Use really clear variable names
3. Avoid defining stuff in the preamble before the test functions. @Dom1L called me out on this in a previous PR, but make sure that if you define something in a preamble it's differentiated from similar stuff within the tests.

@Dom1L can you look at this? Either or post more changes from @nec4, your call.

[brookehus]

@Dom1L I don't think I like the name changes in this commit to random_pseudo_*. I think in this case it's safe to use x for datasets. I am open to using something else, but I almost think dataset makes it more confusing, because it's not like it's a Dataset object. I think I'm going to change to data for now. If we agree on this we can change it in all the tests. @coarse-graining/developers let me know.

[brookehus]

Check out my commits and let me know what you think about data instead!

[Dom1L]

data is more concise than my overly complicated suggestion I agree. Thanks! I like the changes

[Dom1L]

While going through all the tests, I removed all the shadow-names that we had inside functions to be more separated from what we have defined as global variables.

I think that the updated descriptions are helping a lot. The changes look good to me so far!

[Dom1L]

I ran some trials. The distance mean/std test wasn't influenced at all by setting the tolerance lower, so I changed it from 1e-4 to 1e-6 as everything else. The dihedral ones kept on failing ~23/100 times even with 1e-5. At 1e-4 everything is fine, but I guess it might be something to keep in mind

[nec4]

I think I have addresses all comments for test_nnet.py. Let know if anything needs to be modified further.

[brookehus]

@nec4 in test_nnet, we make an instance of geom_stats in line 27.

Then there are some more cases of stats = GeometryStatistics(coords) on line 182 (start of test_prior_callback_order_1) and 208 (start of test_prior_callback_order_2). Are either of these needed or can geom_stats be used?

On line 248 (in test_prior_with_stats_dropout), maybe rename stats to dropout_stats?

[nec4]

I like the idea of replacing all stats with geom_stats name-wise. The instances within the test_prior_callback_order* tests can be removed and the global geom_stats can be used instead. dropout_stats is also now used in the dropout test. These are implemented in b4c79f7 and 4d09b3c respectively.

[brookehus]

Maybe I was the one who put them there when I rearranged things :P. Anyway, I'm good to merge now. @coarse-graining/developers?

[nec4]

I am happy with the changes!

[brookehus]

@nec4 then update your review!