Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrapping the interactions in a structure. #85

Open
JaydevSR opened this issue Aug 12, 2022 · 2 comments
Open

Wrapping the interactions in a structure. #85

JaydevSR opened this issue Aug 12, 2022 · 2 comments

Comments

@JaydevSR
Copy link
Contributor

Right now the interaction in the library are passed down individually as keyword arguments pairwise_inters, specific_inters_list and general_inters. I have a suggenstion that maybe these can be wrapped in a structure like:

const struct Interactions{PW, SF, GN}
    pairwise_inters::PW
    specific_inters_list::SF
    general_inters::GN
end

This can be helpful in grouping together interactions that are meant for a particular simulation/system without having to worry about x3 variables that carry this information. The usefulness of this is not that much when we are talking about simulation with a single type of interactions, but becomes apparent in cases such as Hamiltonian-REMD (which I am trying to implement) where all the replicas have different force fields.

Although we can always use three different vectors to store the different variations of three interactions whenever we want to do such a thing, it seems a bit non-structured (pun intended) way of doing things, when the i'th elements are clearly meant to be used together.

Still, this will be unnecessary if such a situation is rare in practice (which I am not sure about) and also this will increase complexity a bit for simple simulations like with simple LennardJones potential where you will have to do:

inters = Interactions(pairwise_inters=(LennardJones(..), ))

...

Here a workaround can be to write simple convenience functions that set this up for the user:

LennardJonesInteraction(...) = Interactions(pairwise_inters=(LennardJones(..), ))

Please give your views on this.

@jgreener64
Copy link
Collaborator

I am in favour of doing something like this in the long term, it makes conceptual sense to have the force field stored in once struct. It also has the advantage of allowing people to define their own force field types, since dispatch could be controlled when calling forces and potential_energy.

It would be important not to introduce too much boilerplate for users who just want to use one interaction though. It might be possible to allow the new interactions argument to System to take either an Interactions instance as above or an existing pairwise/general interaction, in which case the suggested wrapper types would not be necessary. The interaction could either be wrapped into an Interactions instance in the constructor, which is probably easiest, or could be stored as-is with dispatch being used when calling forces and potential_energy.

@JaydevSR
Copy link
Contributor Author

Wrapping the interactions into an Interaction type inside the constructor seems the best. Also, the user-defined interaction type can even allow for more fields than pairwise_inters, specific_inter_lists and general_inters, where the functions forces and potential_energy will access those fields by iterating over fieldnames and using getfield for that particular type. This seems to give more freedom to anyone wanting to define custom force fields.

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

No branches or pull requests

2 participants