Allowing core modules in lib?

[hugomac...@gmail.com]

Hi all, 

As part of the CZIEOSS4 performance track changes to MDAnalysis, myself and Richard have proposed that the distance routines be able to take both a NumPy array and an AtomGroup (see https://github.com/MDAnalysis/mdanalysis/issues/3708). 

However this violates the rule (guideline, idea?) that modules in `lib` should be NumPy only. and do not allow constructs from `core`.  PR #3730 are the changes in question.

The question is do we want to keep this rule? If so the distance routines (and the `check_coords` decorator) would have to be moved elsewhere. I should note that as part of the changes in #3730 using an AtomGroup is strictly optional and not the default behaviour.

Personally, I don't see the need to strictly separate core and lib. Additionally,  moving the distance routines  (`distances.py`) elsewhere will separate them from a lot of their dependencies (`c_distances.pyx`, `c_distances_openmp.pyx` and `include/calc_distances.h`). 

However, this is my personal opinion and  other people's opinion may differ.  I would love to hear what people think and if they would prefer to maintain the current separation of core and lib, where would be the place to move the distance routines?

Feel free to reply here or on the issue. :)

Cheers

Hugo 

[Oliver Beckstein]

Thanks for starting the discussion, Hugo.

Richard weighed in on the issue stating that making distance calculations use AGs is an important change for the future and that the proposed implementation is the right way to do it.

My understanding is that the changes do not break backwards-compatibility but I’d like this confirmed.

IIRC the reasons for keeping lib free of any MDA-specific imports were

1) avoiding cyclical imports

2) improving interoperability (so that anything in lib could also be used elsewhere)

I don’t think that (2) is a particularly strong argument because importing MDA just for lib still imports all the other core things. We don’t provide a way to only import lib and nothing else.

This leaves (1). Will cyclical imports be an issue with the new proposed changed?

Duplicating code or writing thin wrappers just to uphold the lib/core separation seems messy. Therefore, if cyclical imports are not a problem then I don’t see a reason to keep core data structures strictly separate from lib.

Oliver

--

Oliver Beckstein (he/his/him)

orbe...@gmail.com

[hugomac...@gmail.com]

I should reply here also as well as the issue. 

There are some cyclical import dangers inherent in importing some core modules into lib. As mention in discussion on  https://github.com/MDAnalysis/mdanalysis/issues/3708 this can be avoided one of three ways

1. Use an absolute import (nasty but works)

2. Do a bit of refactoring to fix it for this case (I think moving check coords to its own module in `lib` will fix it)

3.  Stick strictly to the lib=numpy only idea and move distances elsewhere.