MinazoBot commented 4 years ago

	1 Warning
:warning:	There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior.

Generated by :no_entry_sign: Danger

stigrj commented 4 years ago

We get the following error in debug, reproduced on my laptop

make[2]: *** No rule to make target 'lib/libxcfun_d.so.2', needed by 'lib/libmrchem.so'.

@robertodr do you know where the "_d" comes from?

robertodr commented 4 years ago

It's the suffix appended for the debug version. It's not quite clear why it looks for the debug version when it's building it in release...

stigrj commented 4 years ago

I'm not able to locate this error. Should I just change back to debug build of XCFun, or do you want to have a go at it?

robertodr commented 4 years ago

I can have a look tomorrow.

robertodr commented 4 years ago

I cannot reproduce locally using your branch. I am building with ./setup --type=release

stigrj commented 4 years ago

It's when building mrchem in debug while xcfun is release

robertodr commented 4 years ago

It's when building mrchem in debug while xcfun is release

OK, I had completely misunderstood...

robertodr commented 4 years ago

I think the quickest is to remove the DEBUG_POSTFIX property from the xcfun target directly in the XCFun repo. I will release a v2.0.1 once I've tested it with MRChem.

robertodr commented 4 years ago

See dftlibs/xcfun#130

codecov[bot] commented 4 years ago

Codecov Report

Merging #294 into master will decrease coverage by 14.67%. The diff coverage is n/a.

@@             Coverage Diff             @@
##           master     #294       +/-   ##
===========================================
- Coverage   53.92%   39.25%   -14.68%     
===========================================
  Files         105      167       +62     
  Lines        8391    11533     +3142     
===========================================
+ Hits         4525     4527        +2     
- Misses       3866     7006     +3140

Impacted Files	Coverage Δ
...c/qmoperators/one_electron/ElectricFieldOperator.h	`52.63% <0.00%> (-47.37%)`	:arrow_down:
src/mrdft/Functional.h	`60.00% <0.00%> (-40.00%)`	:arrow_down:
src/qmoperators/two_electron/CoulombOperator.h	`68.42% <0.00%> (-31.58%)`	:arrow_down:
src/mrdft/Factory.h	`71.42% <0.00%> (-28.58%)`	:arrow_down:
src/qmoperators/two_electron/XCPotential.h	`80.00% <0.00%> (-20.00%)`	:arrow_down:
src/qmoperators/two_electron/XCOperator.h	`86.66% <0.00%> (-13.34%)`	:arrow_down:
src/chemistry/Element.h	`87.50% <0.00%> (-12.50%)`	:arrow_down:
src/qmoperators/TensorOperator.h	`87.50% <0.00%> (-12.50%)`	:arrow_down:
src/chemistry/Nucleus.h	`88.23% <0.00%> (-11.77%)`	:arrow_down:
src/qmoperators/two_electron/ExchangeOperator.h	`43.75% <0.00%> (-10.10%)`	:arrow_down:
... and 63 more

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 17124d5...cd9d3e3. Read the comment docs.

stigrj commented 4 years ago

Wow, that's a heavy blow on code coverage

robertodr commented 4 years ago

Wow, that's a heavy blow on code coverage

How's that possible? :scream:

stigrj commented 4 years ago

Looks like a lot of files were not included in the coverage before, and for some reason they were included now. I think we need a unit test hackathon

MRChemSoft / mrchem

Update external to official MRCPP-1.2.0 and XCFun-2.0.0 #294

Codecov Report