Update spahm-b #50
Update spahm-b #50
Conversation
| def get_repr(mols, xyzlist, guess, xc=defaults.xc, spin=None, readdm=None, | ||
| pairfile=None, dump_and_exit=False, | ||
| bpath=defaults.bpath, cutoff=defaults.cutoff, omods=defaults.omod, | ||
| elements=None, only_m0=False, zeros=False, split=False, printlevel=0): | ||
|
|
||
| dms = utils.mols_guess(mols, xyzlist, guess, | ||
| xc=xc, spin=spin, readdm=readdm, printlevel=printlevel) | ||
| allvec = bond(mols, dms, bpath, cutoff, omods, | ||
| spin=spin, elements=elements, | ||
| only_m0=only_m0, zeros=zeros, split=split, printlevel=printlevel, | ||
| pairfile=pairfile, dump_and_exit=dump_and_exit) | ||
| return allvec |
There was a problem hiding this comment.
I'm not sure what this function is used for ?
maybe a left-over from before bond()??
There was a problem hiding this comment.
Look in the master branch main() calls get_repr() and get_repr() calls bond().
I think the idea was to have an inner function that doesn't compute the guess, just the representation.
I would rather split spahm(a) in the same manner
| if args.spin: | ||
| if args.merge is False: | ||
| for omod, vec in zip(args.omod, allvec): | ||
| if args.with_symbols: allvec = np.array([(z, v) for v,z in zip(allvec, all_atoms)], dtype=object) | ||
| np.save(args.name_out+'_'+omod, vec) | ||
| else: | ||
| np.save(args.name_out+'_'+'_'.join(args.omod), np.hstack(allvec)) | ||
| allvec = np.hstack(allvec) | ||
| if args.with_symbols: allvec = np.array([(z, v) for v,z in zip(allvec, all_atoms)], dtype=object) | ||
| np.save(args.name_out+'_'+'_'.join(args.omod), allvec) | ||
| else: | ||
| np.save(args.name_out, allvec) | ||
|
|
||
| if args.with_symbols: allvec = np.array([(z, v) for v,z in zip(allvec, all_atoms)], dtype=object) | ||
| np.save(args.name_out+'_'+'_'.join(args.omod), allvec) |
There was a problem hiding this comment.
This part is a bit convoluted!
The default (and only) behaviour in spahm-a is to always return tuple (symbols, reps, omod) and then the main() deals with the merging. and there's no option to save without symbols
- maybe it's worth following spahm-a idea where the specific function for each local rep returns the tuple (with everything separated) and let the
main()function deal with the symbols and the merging options.
What do you think?
There was a problem hiding this comment.
I don't think it should be in main(). Can be in get_repr() that you removed.
The function can be run from a python script so almost everything meaningful should be in functions
| @@ -68,12 +62,15 @@ def main(): | |||
| parser.add_argument('--zeros', action='store_true', dest='zeros', default=False, help='use a version with more padding zeros') | |||
| parser.add_argument('--split', action='store_true', dest='split', default=False, help='split into molecules') | |||
| parser.add_argument('--merge', action='store_true', dest='merge', default=True, help='merge different omods') | |||
| parser.add_argument('--symbols', action='store_true', dest='with_symbols', default=False, help='if save tuples with (symbol, vec) for all atoms') | |||
There was a problem hiding this comment.
maybe make it a --nosymbols option instead. (see comment below)
ecpargument for CLIparser--symboloption vs spahm-a(keep or remove)(Closes #47)