search

LeMonADE-project / LeMonADE

Library for Monte Carlo Simulation applying the Bond Fluctuation Model
Other
3 stars 11 forks source link

FeatureNNInteraction lattice type #64

Open MartinWenge opened 7 years ago

MartinWenge commented 7 years ago

I'm talking about this file: FeatureNNInteraction.h When making the simple examples for the paper i stumpled over some type issues: The lattice type is defined as uint8_t, interaction/probability lookup goes from 0 to 255 which is the range of uint8_t, but the setter/getter are dealing with int32_t: void setNNInteraction(int32_t typeA,int32_t typeB,double energy); , double getNNInteraction(int32_t typeA,int32_t typeB) const;, and double getProbabilityFactor(int32_t typeA,int32_t typeB) const;

Same for FeatureNNInteractionBcc i guess. This is not a mayjor point, but its not clean and might cause some untraceable type conversion problems. Where to start changes? Which type to adjust? What about the FeatureAttributiesType? Some much to worry about...

hrabbel commented 7 years ago

I would suggest to leave the interfaces of the public member functions (getNNInteraction, setNNInteraction) consistent with the type used in FeatureAttributes. That way, the user can directly use the attribute as argument to those functions. Checks for consistency with the lattice type (uint8) are already done within FeatureNNInteraction.

Still, one could change the type used in FeatureAttributes to unsigned. I think I would like that option. I think positive attributes are not too much of a restriction, and this would eliminate the unigned-signed conversion of getNNInteraction.

MartinWenge commented 5 years ago

We should reconsider this discussion after the release 2.1. where FeatureAttributes are provided with an arbitrary type.