Various small changes

[davidbbeyer]

@pm-blanco I have addressed the points raised by the reviewer:

• In all samples and tests, instances of reaction ensemble objects have been renamed to cpH and grxmc to be more descriptive
• The SEED has been renamed to seed
• The activity coefficient is now a mandatory argument of GCMC and G-RxMC, i.e. it even needs to be provided for an ideal system. Sample script and tests have been updated accordingly.
• The charge q of a particle was changed to the charge number z

Furthermore, I fixed some typos I found.

[pm-blanco]

@davidbbeyer in general, it looks good to me. However, there is a few inconsistencies that still remain between q and z:

• In pmb.df we store z in the multindex columns ('state_one','charge') and, for acid and basic particles, in ('state_two','charge'). I believe that the naming is a bit unfortunate because it implies that we store there the charge (which should have units) instead than the charge number. I suggest renaming them to ('state_one','z') and in ('state_two','z')
• In pmb.get_charge_map and get_metal_ions_charge_map, we have the same issue, we state that we return a map with the charge of the particles but it actually returns the charge number. I suggest to rename these functions to pmb.get_charge_number_map and get_metal_ions_charge_number_map for clarity and addapt the docstrings accordingly.
• In pmb.set_particle_acidity, default_charge should be probably replaced by z and the docstring should be adapted to explain that it expects the charge number
• We should consider if pmb.calculate_net_charge should return the net charge with units, otherwise we are again being inconsistent with the units.

I think that we should address points at least points 1-3 in this PR. Point 4 we can postpone for its own PR if you do not have the time right now, since it might require more work. Since you need to change pmb.get_charge_map, it would be great if you could do some small unit tests for that function so we can continue making progress with #58.

[davidbbeyer]

@pm-blanco I have implemented the suggested changes and added a test.

[davidbbeyer]

• I have removed the print statement
• pmb.calculate_net_charge now has an additional boolean argument dimensionless that allows one to set if it should return the charge with a dimension or dimensionless. In my opinion, this is a clean option, since removing the units from the nested dictionaries is cumbersome.
• Unrelated, but I also added unit tests for pymbe_library.generate_coordinates_outside_sphere() and pymbe_library.generate_random_points_in_a_sphere() to increase the code coverage

[pm-blanco]

Thank you for the additional unit tests @davidbbeyer.

• generate_coordinates_outside_sphere is missing a couple of unit tests to have complete coverage:

def generate_coordinates_outside_sphere(self, center, radius, max_dist, n_samples):
        if not radius > 0: 
            raise ValueError (f'The value of {radius} must be a positive value')
        if not radius < max_dist:
            raise ValueError(f'The min_dist ({radius} must be lower than the max_dist ({max_dist}))')

the sanity checks (the lines below the if stataments) are not covered. For completion, could you please provide a couple of tests that check that the function raises a ValueError? You can use np.testing.assert_raises, find some examples in testsuite/define_and_create_molecules_unit_tests.py

• In the docstring of generate_random_points_in_a_sphere, it says that the algorithm is from here. I believe that was true for some older version of that function but it is not true anymore, since you modified it. If that is correct, can you please remove that note from the docstring?

• I think that adding the dimensionless feature to calculate_net_charge is a good compromise solution between being consistent with the units and having a convenient way of handling it when working in reduced units. However, we need to add a unit test in testsuite/calculate_net_charge_unit_test.py to check the case with dimensionless = True to cover the corresponding lines and also to ensure that it does not break in the future.

[davidbbeyer]

I have implemented your suggestions.