search

lcmd-epfl / Q-stack

Stack of codes for dedicated pre- and post-processing tasks for Quantum Machine Learning (QML)
MIT License
14 stars 5 forks source link

Global SPAHM gradient #10

Closed briling closed 1 year ago

rlaplaza commented 1 year ago

I assume you went for a 2 step formula for the finite differences because it was slow otherwise. If its not slow (small molecule) we should probably use a four point stencil (i.e. https://en.wikipedia.org/wiki/Five-point_stencil) for the numerical gradient.

Also, in the test, myfunc is evil (evil name, evil use of mol that is not passed as argument). Otherwise the reactors look good. And tests pass.

briling commented 1 year ago

Thanks!

I assume you went for a 2 step formula for the finite differences because it was slow otherwise. If its not slow (small molecule) we should probably use a four point stencil (i.e. https://en.wikipedia.org/wiki/Five-point_stencil) for the numerical gradient.

I'm not sure it makes sense given the numerical gradient is only for this test. Why make it more complicated?

Also, in the test, myfunc is evil (evil name, evil use of mol that is not passed as argument). Otherwise the reactors look good. And tests pass.

I've changed the evil name but can't find where's the problem with mol...

rlaplaza commented 1 year ago

The 2 step finite differences can be bad so the same test could potentially fail for a different molecule (for example).

Issue with mol is in

mymol.atom = [ (mol.atom_symbol(i), r[i]) for i in range(mol.natm)]

which takes mol from outside the function. This is also a liability.

briling commented 1 year ago

Issue with mol is in

mymol.atom = [ (mol.atom_symbol(i), r[i]) for i in range(mol.natm)]

which takes mol from outside the function. This is also a liability.

This line is inside build_mol() which takes mol as the first argument

rlaplaza commented 1 year ago

Yes, you fixed it. Lets push! GJ!