theochem / horton

HORTON: Helpful Open-source Research TOol for N-fermion systems
http://theochem.github.io/horton/
GNU General Public License v3.0
92 stars 40 forks source link

obasis.compute_grid_orb_gradient_exp: Orbital Gradients on a grid #206

Closed sfias closed 7 years ago

sfias commented 7 years ago

I need the gradients of the molecular orbitals on a grid for a project with some students. I think it would be easier for them to have this included in HORTON instead of using my personal copy.

This lets us use: obasis.compute_grid_orb_gradient_exp(exp, points, orbs))

    Parameters
    ----------

    exp : DenseExpansion
        Orbitals.
    points : np.ndarray, shape=(npoint, 3), dtype=float
        Cartesian grid points.
    iorbs : np.ndarray, shape=(n,), dtype=int
        Indexes of the orbitals to be computed.
    output : np.ndarray, shape=(npoint, n, 3), dtype=float
        An output array. The results are added to this array. When not given, an
        output array is allocated.

    Returns
    -------
    output : np.ndarray, shape=(npoint, n, 3), dtype=float
        the output array. (It is allocated when not given.)
tovrstra commented 7 years ago

Thanks for the PR. Looks good but there are some style issues reported by Travis-CI. Can you check? It would make sense for me to disable some of the more nittygritty code checks, i.e.

sfias commented 7 years ago

@tovrstra , I disabled the checks you mentioned, updated the doc/tech_dev_checklist.rst and finally managed to get it passed Travis.

tovrstra commented 7 years ago

Wonderful! I'll take a closer look soon.

kimt33 commented 7 years ago

buildkite (git) seems to raise the error "fatal: reference is not a tree: " or "PR from unapproved repo sfias."?

matt-chan commented 7 years ago

Yep. It has to do with the way buildkite keeps our server from running anybody's code. When a PR is made, buildkite runs the PR, even if it replaces the Horton code entirely with an email spammer. Travis does this too, but it uses a virtual machine I think. We don't have that luxury because our Mac and windows builders run it directly.

So to avoid that situation, there's an approved list of repos that we can make PRs from. In practice, making PRs from quantum elephant is the long term solution. I can add sfias to the approved list for now though.

On Tue, Mar 28, 2017, 5:28 PM Taewon D. Kim notifications@github.com wrote:

buildkite (git) seems to raise the error "fatal: reference is not a tree: " or "PR from unapproved repo sfias."?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/theochem/horton/pull/206#issuecomment-289945606, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_-Nb9BsFTv2D2ov3qsbU27_n5qKxjuks5rqaWygaJpZM4MBxLO .

-- Matt

Sent from my phone

tovrstra commented 7 years ago

Looks good. @sfias: can you explain the purpose of the @cond Doxygen_Suppress lines?

sfias commented 7 years ago

On Travis, Doxygen complains at line 327 of horton/gbasis/fns.h about poly_work{0.0}, offset(0), offset_l1(0) and offset_h1(0) not being documented (on my mac I don't get any complaints). They are, however, explained at lines 346 to 349. When I do try to document it, it complains about it being documented wrong. The code is consistent with the way e.g. GB1ExpGridOrbitalFn, and GB1DMGridGradientFn are coded (for which I guess the same problem should occur?!). I tried to fix this and asked around in the lab, but no-one here seems to know how to fix it (I haven't asked Matt since he's in the US). To avoid these warnings, I suppressed the Doxygen warnings for this code. If you want I can remove it to let you see where Travis complains...

tovrstra commented 7 years ago

Thanks for explaining. I'll see if there is a nice way around it.

sfias commented 7 years ago

Thanks for looking at it, I think Doxygen is being too picky here, but I didn't want to switch off the test completely.

kimt33 commented 7 years ago

It seems that the Travis cannot meet some of HORTON's dependencies. It is using Doxygen 1.7.6 which is quite a bit older than the required 1.8.6. All of its warnings:

warning: ignoring unsupported tag `MARKDOWN_SUPPORT       =' at line 283, file doxygen.conf
warning: ignoring unsupported tag `AUTOLINK_SUPPORT       =' at line 291, file doxygen.conf
warning: ignoring unsupported tag `EXTRACT_PACKAGE        =' at line 413, file doxygen.conf
warning: ignoring unsupported tag `SHOW_GROUPED_MEMB_INC  =' at line 510, file doxygen.conf
warning: ignoring unsupported tag `USE_MDFILE_AS_MAINPAGE =' at line 882, file doxygen.conf
warning: ignoring unsupported tag `SOURCE_TOOLTIPS        =' at line 938, file doxygen.conf
warning: ignoring unsupported tag `HTML_EXTRA_STYLESHEET  =' at line 1071, file doxygen.conf
warning: ignoring unsupported tag `HTML_INDEX_NUM_ENTRIES =' at line 1140, file doxygen.conf
warning: ignoring unsupported tag `MATHJAX_FORMAT         =' at line 1420, file doxygen.conf
warning: ignoring unsupported tag `MATHJAX_CODEFILE       =' at line 1448, file doxygen.conf
warning: ignoring unsupported tag `EXTERNAL_SEARCH        =' at line 1497, file doxygen.conf
warning: ignoring unsupported tag `SEARCHENGINE_URL       =' at line 1508, file doxygen.conf
warning: ignoring unsupported tag `SEARCHDATA_FILE        =' at line 1516, file doxygen.conf
warning: ignoring unsupported tag `EXTERNAL_SEARCH_ID     =' at line 1524, file doxygen.conf
warning: ignoring unsupported tag `EXTRA_SEARCH_MAPPINGS  =' at line 1534, file doxygen.conf
warning: ignoring unsupported tag `LATEX_EXTRA_FILES      =' at line 1628, file doxygen.conf
warning: ignoring unsupported tag `GENERATE_DOCBOOK       =' at line 1809, file doxygen.conf
warning: ignoring unsupported tag `DOCBOOK_OUTPUT         =' at line 1817, file doxygen.conf
warning: ignoring unsupported tag `EXTERNAL_PAGES         =' at line 1990, file doxygen.conf
warning: ignoring unsupported tag `DIA_PATH               =' at line 2025, file doxygen.conf
warning: ignoring unsupported tag `UML_LIMIT_NUM_FIELDS   =' at line 2119, file doxygen.conf
warning: ignoring unsupported tag `DIAFILE_DIRS           =' at line 2231, file doxygen.conf

I've updated Doxygen to 1.8.6 and the trapdoor_doxygen passes (as it did in Stijn's computer). See Issue #222.

kimt33 commented 7 years ago

@sfias I think that PR #223 should fix your doxygen problem and you can remove the @cond Doxygen_Suppress. Turns out you were right this whole time.

sfias commented 7 years ago

I rebased after PR #223 and removed the @cond Doxygen_Suppress. It passed travis. Thanks @kimt33 !

tovrstra commented 7 years ago

I've looked into the math before and it was back then, so I guess it should still be good. The code style issues are all sorted out, so fine by me too.

tovrstra commented 7 years ago

P.S. Any idea why buildkite is not responsive?

matt-chan commented 7 years ago

The server is sitting in our coffee space, unplugged. We're trying to get it moved to the datacenter because it's a bit loud. Hopefully I can make it happen before I leave for Belgium!

So the code is good then. I'll merge it in.