franciscoadasme / chem.cr

Library for dealing with computational chemistry files
https://franciscoadasme.github.io/chem.cr/
MIT License
23 stars 1 forks source link

Add Spatial::Point type #199

Open franciscoadasme opened 1 year ago

franciscoadasme commented 1 year ago

Right now, Spatial::Vec3 can represent both a point in space or a "vector" between two points, and a method may require either a point or vector so there is an inherent ambiguity. The docs should state such detail but it could be easily missed. For instance, Spatial.angle has two overloads:

def Spatial.angle(a : Vec3, b : Vec3) : Float64
def Spatial.angle(a : Vec3, b : Vec3, c : Vec3) : Float64

The first overload computes the angle between two vectors as expected, but the second one assumes that the arguments represents points in space. Passing three "vectors" makes no sense, which may go undetected leading to incorrect results.

Even though using Spatial::Vec3 everywhere provides flexibility, it may cause some issues in the long run. So adding an explicit type that resembles the expected data more closely may help to avoid such ambiguity.

franciscoadasme commented 1 year ago

The gemmi library for macromolecular crystallography even has Position and Fractional types to differentiate between Cartesian and fractional positions. Such distinction may be useful for methods such as #wrap, where Vec3#wrap assumes that they're fractional coordinates, but Parallelepiped#wrap(Vec3) expects Cartesian coordinates. However, it may be cumbersome to work with two distinct types for coordinates, e.g., What would be the type declaration for CoordinatesProxy?, as CoordinatesProxy#to_cart! and #to_fract! methods would change the coordinate type.