theochem / cgrid

C++ version of horton (2.x) grid functionality
GNU General Public License v3.0
4 stars 1 forks source link

First step toward scalar function classes #22

Closed tovrstra closed 6 years ago

codecov[bot] commented 7 years ago

Codecov Report

Merging #22 into master will increase coverage by 9.45%. The diff coverage is 93.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #22      +/-   ##
==========================================
+ Coverage   80.65%   90.11%   +9.45%     
==========================================
  Files           7       11       +4     
  Lines         305     1234     +929     
  Branches       30      134     +104     
==========================================
+ Hits          246     1112     +866     
- Misses         57      114      +57     
- Partials        2        8       +6
Impacted Files Coverage Δ
qcgrids/scalarfns.h 100% <100%> (ø)
qcgrids/scalarfns.cpp 100% <100%> (ø)
python-qcgrids/qcgrids/test_qcgrids.py 100% <100%> (ø) :arrow_up:
python-qcgrids/qcgrids/ext.pyx 56.52% <70.39%> (+32.52%) :arrow_up:
qcgrids/tests/ridders.h 71.87% <71.87%> (ø)
qcgrids/tests/test_scalarfns.cpp 99.45% <99.45%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 1f70c00...ab12b16. Read the comment docs.

tovrstra commented 7 years ago

Note: work in progress...

tovrstra commented 7 years ago

To do list:

tovrstra commented 7 years ago

@matt-chan @FarnazH This is a rather big and boring first step in qcgrids. These scalar functions are useful when defining radial grids and when building models of densities/potentials on radial grids. This is loosely based on similar code in horton.grid.rtransform.* and horton.grid.cubic_spline.*. The code is somewhat simplified and kept more general.

tovrstra commented 7 years ago

All was just rebased.

matt-chan commented 7 years ago

@tovrstra, I'm back in Canada now =)! Is this the complete code? or is it still missing the lines from before?

tovrstra commented 7 years ago

I think I found them. Have a good time home!

tovrstra commented 7 years ago

@matt-chan they are complete again. Seems to be working.

tovrstra commented 7 years ago

@matt-chan @FarnazH ping. would you still like to take a look in the near future? I'm fine either way. If not, I'll merge soon.

FarnazH commented 7 years ago

@tovrstra Thanks for your message. I will take a look at it on this weekend, but if you need to merge it earlier please feel free to do so.

tovrstra commented 7 years ago

No rush. I'm still occupied by plenty of other things.

tovrstra commented 7 years ago

No problem. Regarding the docstrings, functions in extension modules do not have function signatures by default. You either write them manually on the first line of the docstring or you can instruct Cython to do this for you. I'll enable that Cython option for now.

matt-chan commented 7 years ago

Sounds good! Thanks Toon!

On Mon, 23 Oct 2017 at 10:50 Toon Verstraelen notifications@github.com wrote:

No problem. Regarding the docstrings, functions in extension modules do not have function signatures by default. You either write them manually on the first line of the docstring or you can instruct Cython to do this for you. I'll enable that Cython option for now.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/theochem/qcgrids/pull/22#issuecomment-338684297, or mute the thread https://github.com/notifications/unsubscribe-auth/AA_-Nc2ggw7h91pTG12nVZvqY6IskLsmks5svKefgaJpZM4PMn49 .

-- Matt

Sent from my phone

tovrstra commented 6 years ago

Small note: I fixed the issues with -Werror causing lots of troubles for the unit tests due to weird tricks used in gtest. Not our fault. -Werror is only disabled for the unit tests, not for the rest of the code.