-
Notifications
You must be signed in to change notification settings - Fork 527
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
Enable setting test size individually for each system #267
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The unittest was not passing here:
deepmd-kit/source/tests/test_deepmd_data_sys.py
Lines 79 to 100 in 59d780c
def test_get_test(self): | |
batch_size = 3 | |
test_size = 2 | |
ds = DeepmdDataSystem(self.sys_name, batch_size, test_size, 2.0) | |
ds.add('test', self.test_ndof, atomic = True, must = True) | |
ds.add('null', self.test_ndof, atomic = True, must = False) | |
sys_idx = 0 | |
data = ds.get_test(sys_idx=sys_idx) | |
self.assertEqual(list(data['type'][0]), list(np.sort(self.atom_type[sys_idx]))) | |
self._in_array(np.load('sys_0/set.002/coord.npy'), | |
ds.get_sys(sys_idx).idx_map, | |
3, | |
data['coord']) | |
self._in_array(np.load('sys_0/set.002/test.npy'), | |
ds.get_sys(sys_idx).idx_map, | |
self.test_ndof, | |
data['test']) | |
self.assertAlmostEqual(np.linalg.norm(np.zeros([self.nframes[sys_idx]+2, | |
self.natoms[sys_idx]*self.test_ndof]) | |
- | |
data['null'] | |
), 0.0) |
I am taking a look at it, just haven't figured out where the problem is yet. |
Everything should be in order now. |
The new features of the code will be merge to devel branch. |
clear the confusion caused by adding python style comments to json file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing comma
Co-authored-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Co-authored-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Enable setting test size individually for each system
When one is dealing with multiple systems which have very distinct numbers of frames setting number of tests as only one integer which is same for all systems is rather restraining. For example consider two systems:
Setting number of tests to 10 should be ideal for the second system but is far too low for the first one. There is no good solution to this situation, since increasing test number to say 100 would be appropriate for the first system but wrong for the second.
The change I propose simply allows to set
numb_test
parameter as a list with separate value for each system or as a string specifying a percentage of the systems frames that should be used for testing.In the first case, number of tests could be specified in json as
numb_test : [1000, 10]
or"10%"
which would amount to 10% of testing frames for each of the systems respectively in both of the settings.This requires addition of only a few lies of code and minimal new logic, all changes are commented in the code.
I am pushing this to devel branch, since I don't know your workflow, but if you accept it can be merged directly to master too as it would be nice to see it in conda and docker as soon as possible.
I also made a few minor changes to README which I think serve to better clarify the purpose of json settings.