Enable tests in build #113

Zaharid · 2018-02-28T12:10:33Z

This implements some but not all of the points in #95. In order to be able to merge it, we require a low precision theory that we can use it tests. Currently, it predictably fails with:

  [utils] error: Could not open /home/zah/anaconda3/conda-bld/
  nnpdf_1519818938025/
  _test_env_placehold_placehold_placehold_placehold_placehold_placehold_placeh-
  old_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pl-
  acehold_placehold_placehold_placehold_placehold_place/share/NNPDF/data/
  theory_53/fastkernel/FK_NMC.dat

It is also missing enabling coverage reports, but that's out of scope here.

Zaharid · 2018-03-01T19:09:36Z

@nhartland, could you produce one theory with low precision and a couple of experiments? Then we can download it during the testing phase and use it to check fktables and whatnot.

nhartland · 2018-03-06T16:50:28Z

How big would be acceptable? We already have theory 162 which is super-low precision, but it includes all sets. It's coming in at 125 Mb/

nhartland · 2018-03-06T16:52:07Z

As opposed to the usual ~5 GB.

Zaharid · 2018-03-06T16:55:51Z

Could we have a subset of that with a couple of experiments? In any case 125Mb is less than some conda packages, so it's probably fine.

nhartland · 2018-03-06T17:10:34Z

We can do, but I'm lazy and adding theories is a pain.

Zaharid · 2018-03-06T17:14:16Z

Let's try with 162 and see how long takes for travis to kick as out.

Zaharid · 2018-03-08T10:50:36Z

Great, getting two different build failures on linux and mac. I really have no patience for understanding how linking in mac works but it is really annoying me how absurd it is.

https://travis-ci.com/NNPDF/nnpdf/jobs/113751056
https://gitlab.cern.ch/NNPDF/nnpdf/-/jobs/1112656

Zaharid · 2018-03-08T11:11:04Z

This now works on linux (albeit it is somewhat absurd to run tests in release mode). @nhartland I would appreciate some help with the Mac failure, which looks frankly nonsensical.

nhartland · 2018-03-08T11:57:28Z

This error?
dyld: Symbol not found: _cblas_dgemv ?

Zaharid · 2018-03-08T11:58:32Z

Yes. Also, apparently I haven't fixed travis so it refuses to build anything....

nhartland · 2018-03-08T12:00:08Z

What is the deal with travls? I keep getting emails saying we're running out of trials. Is this something we'll need to pay for at some point?

Zaharid · 2018-03-08T12:09:01Z

Apparently I have it for free, but the repo must belong to me. I though it would be enough to put my credentials.

Zaharid · 2018-03-08T13:44:05Z

I have set up travis on my fork. I have also sent an email asking if we could have it for NNPDF. For now a solution would be:

git remote set-url origin --push --add git@github.com:Zaharid/nnpdf.git

which causes to push both to the NNPDF and to my repository, while pulling is always from NNPDF. This is not ideal since e.g. we don't get the flag in the PR.

Zaharid · 2018-03-08T13:45:04Z

The build of the test keeps failing:

https://travis-ci.com/Zaharid/nnpdf/builds/67740102?utm_source=email&utm_medium=notification

I suppose some permutation of the options here would fix it:

https://cmake.org/Wiki/CMake_RPATH_handling

but I have ran out of patience.

Zaharid · 2018-03-12T21:12:19Z

Travis is working now, so it would be good to have it do something useful!

scarrazza · 2018-03-12T22:11:15Z

Did they reply to your email?

nhartland · 2018-03-15T20:35:09Z

Assigned #113 <#113> to @nhartland <https://github.com/nhartland>.

Haha, cheers!

…

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#113 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AE_NDFEwyVPvU5Qi2X6yPeyYemBIdu3-ks5teqcPgaJpZM4SWjWk>.

nhartland · 2018-03-19T12:50:46Z

So I'm not even sure how to reproduce this error let alone try and resolve it.
Do I set up a conda dev env and try to install this branch to it? Which part is failing?

I can't see the travis output for the nnpdf version in your repo:

https://travis-ci.com/Zaharid/nnpdf/builds/67740102?utm_source=email&utm_medium=notification

because I don't have access to it

Zaharid · 2018-03-21T13:46:17Z

On the travis logs, there is:

+LDFLAGS=-Wl,-pie -Wl,-headerpad_max_install_names -Wl,-dead_strip_dylibs -Wl,-rpath,/Users/travis/miniconda3/conda-bld/nnpdf_1521626381535/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib -L/Users/travis/miniconda3/conda-bld/nnpdf_1521626381535/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/lib

In particular, -dead_strip_dylibs looks like a candidate to be causing the trouble.

nhartland · 2018-03-21T13:50:06Z

Haha yes indeed. I'll try just overwriting LDFLAGS without that.

nhartland · 2018-03-21T15:41:33Z

So now there is a different error. I don't know if overwriting the LDFLAGS without that flag has fixed the problem or if we've just covered it up with another problem, but let's be optimistic!

The error now is an rpath one which I suspect would be fixed by installing the tests before running them (if that makes sense?). You've set SET(CMAKE_INSTALL_RPATH_USE_LINK_PATH TRUE) in the cmake conf, which should setup the rpaths correctly upon installation. This seems to work for the normal binaries after all.

Zaharid · 2018-03-21T20:37:37Z

Seems like this worked!

nhartland · 2018-03-22T11:18:32Z

Are these flags pretty much hard coded into conda then, not something we can trivially remove from some config? Overwriting the LDFLAGS is probably asking for trouble further down the line.

Maybe something as simple as running LDFLAGS through sed and cutting out that one flag would be marginally more robust.

Zaharid · 2018-03-22T11:21:23Z

I was going to do the sed thing.

Zaharid · 2018-03-22T17:49:32Z

So my understanding of the issue with Mac and conda is the following:

In order to link to something that is not in the default search paths, you need to pass the rpath argument to the linker with the path where that library is located. Conda does this by default, but Makefiles need to by mindful of the environment variables.
In addition conda passes the -dead_strip_dylibs which breaks everything for reason I don't understand or care for particularly. We should strip this path from the conda environment. The most straight forward way is to just remove it with sed.

@nhartland, can you get a working dev environment with that information?

This makes self testing easier.

Set up the requirement environment to: - Be able to test validphys with pytest. - Be able to compile and run the catch test, with all the debug flags enabled. This has the added benefit of testing that the debug build actually works.

Apparently these are not defined and we want to have the undefined check.

This requires updating all the regression tables with the new theory. We also put all the tests in a file so that we have clear control over the order. In particular we want python test first so that we download theories and pdfs.

One of the many useless missfeatures of setuptools is that it doesn't copy all of the files by default.

I am only half sure this is a good idea...

If it compiles the library maybe it will compile the tests?

As per https://cmake.org/Wiki/CMake_RPATH_handling

Zaharid · 2018-03-23T15:04:39Z

I'd like to merge this because it is annoying to write tests that depend on it otherwise.

nhartland

Sure, sorry, obviously fine by me. I'll see if I can finally resolve #98.

Dijets

Zaharid force-pushed the tests branch 3 times, most recently from 769836e to 6b33fa8 Compare February 28, 2018 13:27

Zaharid mentioned this pull request Mar 6, 2018

Add regression tests for theory matrices #117

Closed

Zaharid mentioned this pull request Mar 7, 2018

[Speculative] Modifications used for reweighting #120

Merged

Zaharid force-pushed the tests branch 2 times, most recently from f8dcfac to dc91b55 Compare March 8, 2018 10:17

Zaharid closed this Mar 8, 2018

Zaharid reopened this Mar 8, 2018

Zaharid assigned nhartland Mar 15, 2018

Zaharid added urgent help wanted Extra attention is needed labels Mar 15, 2018

Zaharid force-pushed the tests branch 2 times, most recently from 5db2a30 to 457557d Compare March 21, 2018 16:43

Zaharid force-pushed the tests branch from b8fb573 to 1694ea6 Compare March 22, 2018 17:03

Zaharid changed the title ~~[WIP] Enable tests in build~~ Enable tests in build Mar 22, 2018

Zaharid added 12 commits March 22, 2018 18:06

Move all python test files to validphys

e075bee

This makes self testing easier.

Remove outdated test file

19ec5f5

Run python and cpp tests with conda

85f5c5f

Set up the requirement environment to: - Be able to test validphys with pytest. - Be able to compile and run the catch test, with all the debug flags enabled. This has the added benefit of testing that the debug build actually works.

Fix CPPFLAGS on Mac

b790f46

Apparently these are not defined and we want to have the undefined check.

Use theory 162 in tests

47c0be9

This requires updating all the regression tables with the new theory. We also put all the tests in a file so that we have clear control over the order. In particular we want python test first so that we download theories and pdfs.

Formatting

b5c7b2f

Use theory 162 in cpp tests

7e48b20

Add csv files to setup.py

169f90b

One of the many useless missfeatures of setuptools is that it doesn't copy all of the files by default.

Add pctserver credentials

3d056b5

I am only half sure this is a good idea...

Try to run c++ tests in release mode

ff3e974

If it compiles the library maybe it will compile the tests?

Attempt to fix mac build

55f5ad2

As per https://cmake.org/Wiki/CMake_RPATH_handling

Try surgical change

ac6cb56

Zaharid force-pushed the tests branch from aed6c6a to ac6cb56 Compare March 22, 2018 18:12

nhartland approved these changes Mar 23, 2018

View reviewed changes

Zaharid merged commit 14c63c7 into master Mar 23, 2018

nhartland deleted the tests branch April 3, 2018 17:54

Zaharid mentioned this pull request Oct 16, 2018

dyld: _cblas_dgemv Error when executing filter #306

Closed

voisey pushed a commit that referenced this pull request Aug 26, 2020

Merge pull request #113 from NNPDF/DIJETS

e56277c

Dijets

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable tests in build #113

Enable tests in build #113

Zaharid commented Feb 28, 2018

Zaharid commented Mar 1, 2018

nhartland commented Mar 6, 2018

nhartland commented Mar 6, 2018

Zaharid commented Mar 6, 2018

nhartland commented Mar 6, 2018

Zaharid commented Mar 6, 2018

Zaharid commented Mar 8, 2018

Zaharid commented Mar 8, 2018

nhartland commented Mar 8, 2018

Zaharid commented Mar 8, 2018

nhartland commented Mar 8, 2018

Zaharid commented Mar 8, 2018

Zaharid commented Mar 8, 2018

Zaharid commented Mar 8, 2018 •

edited

Loading

Zaharid commented Mar 12, 2018

scarrazza commented Mar 12, 2018

nhartland commented Mar 15, 2018 via email

nhartland commented Mar 19, 2018

Zaharid commented Mar 21, 2018

nhartland commented Mar 21, 2018

nhartland commented Mar 21, 2018

Zaharid commented Mar 21, 2018 via email

nhartland commented Mar 22, 2018

Zaharid commented Mar 22, 2018

Zaharid commented Mar 22, 2018 •

edited

Loading

Zaharid commented Mar 23, 2018

nhartland left a comment

Enable tests in build #113

Enable tests in build #113

Conversation

Zaharid commented Feb 28, 2018

Zaharid commented Mar 1, 2018

nhartland commented Mar 6, 2018

nhartland commented Mar 6, 2018

Zaharid commented Mar 6, 2018

nhartland commented Mar 6, 2018

Zaharid commented Mar 6, 2018

Zaharid commented Mar 8, 2018

Zaharid commented Mar 8, 2018

nhartland commented Mar 8, 2018

Zaharid commented Mar 8, 2018

nhartland commented Mar 8, 2018

Zaharid commented Mar 8, 2018

Zaharid commented Mar 8, 2018

Zaharid commented Mar 8, 2018 • edited Loading

Zaharid commented Mar 12, 2018

scarrazza commented Mar 12, 2018

nhartland commented Mar 15, 2018 via email

nhartland commented Mar 19, 2018

Zaharid commented Mar 21, 2018

nhartland commented Mar 21, 2018

nhartland commented Mar 21, 2018

Zaharid commented Mar 21, 2018 via email

nhartland commented Mar 22, 2018

Zaharid commented Mar 22, 2018

Zaharid commented Mar 22, 2018 • edited Loading

Zaharid commented Mar 23, 2018

nhartland left a comment

Choose a reason for hiding this comment

Zaharid commented Mar 8, 2018 •

edited

Loading

Zaharid commented Mar 22, 2018 •

edited

Loading