Bug 242672

Summary: science/py-scikit-learn: Fails due to unresolved symbols (py36-scikit-learn-0.20.3_1)
Product: Ports & Packages Reporter: Andy Mender <andymenderunix>
Component: Individual Port(s)Assignee: Wen Heping <wen>
Status: Closed FIXED    
Severity: Affects Many People CC: demon, koobs, python, wen
Priority: --- Flags: koobs: maintainer-feedback+
Version: Latest   
Hardware: amd64   
OS: Any   
See Also: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=194683
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238718
Bug Depends on: 238718    
Bug Blocks:    

Description Andy Mender 2019-12-16 21:48:55 UTC
This issue was reported for the Python 2.7 flavor of this port a while back here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=194683

I get the following traceback:
  File "scripts/svm.py", line 13, in <module>
    from sklearn.svm import SVC
  File "/usr/local/lib/python3.6/site-packages/sklearn/svm/__init__.py", line 13, in <module>
    from .classes import SVC, NuSVC, SVR, NuSVR, OneClassSVM, LinearSVC, \
  File "/usr/local/lib/python3.6/site-packages/sklearn/svm/classes.py", line 4, in <module>
    from .base import _fit_liblinear, BaseSVC, BaseLibSVM
  File "/usr/local/lib/python3.6/site-packages/sklearn/svm/base.py", line 8, in <module>
    from . import libsvm, liblinear
ImportError: /usr/local/lib/libcblas.so.2: Undefined symbol "cgemv_"

Not sure whether it's worth addressing, though, since apparently a new version of science/py-scikit-learn is on its way here: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238718

If there is no interest in maintaining scikit-learn, I can claim maintainership of the science/py-scikit-learn port beginning of next year since I actually do use scikit-learn for my projects. In a separate report, of course.
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2019-12-17 01:23:04 UTC
Thank you for the report Andy

Does the 0.12.2 update in bug 238718 fail in the same way?
Comment 2 Kubilay Kocak freebsd_committer freebsd_triage 2019-12-17 01:24:43 UTC
Also, does the py27 version of the current port fail in the same way?

^Triage: Request feedback from committer (resolver) of bug 194683
Comment 3 Kubilay Kocak freebsd_committer freebsd_triage 2019-12-17 01:35:00 UTC
Sorry, should have bundled all of these questions together:

In addition to the questions in comment 1 and comment 2

After rebuilding with `LDFLAGS=-lblas make deinstall reinstall clean`, does it fail in the same way?
Comment 4 Andy Mender 2019-12-18 09:48:29 UTC
> Does the 0.12.2 update in bug 238718 fail in the same way?

You meant the 0.22 update, yes? I haven't tested it yet.

> Also, does the py27 version of the current port fail in the same way?

I changed one of the scripts demonstrating this bug to fit the Python 2.7 syntax and it still generates the very same error. Would you like me to create a sample bundle with the input data set + the error-triggering Python code so that it can be used in tests?

> After rebuilding with `LDFLAGS=-lblas make deinstall reinstall clean`, does it fail in the same way?

I will test this and the 0.22 version as soon as I get the SVN ports tree onto this machine. So far I was only using the pre-built packages.
Comment 5 Andy Mender 2019-12-19 22:27:17 UTC
> I will test this and the 0.22 version as soon as I get the SVN ports tree onto this machine. So far I was only using the pre-built packages.

I installed all dependencies from packages and built the science/py-scikit-learn port (version 0.22) for Python 3.7 (Python 3.6 is no longer the default FLAVOR). from the Ports Collection. After running a couple of fairly minimal classification models with scikit-learn, I can confirm that it works as intended and does not throw the error mentioned before.

> After rebuilding with `LDFLAGS=-lblas make deinstall reinstall clean`, does it fail in the same way?

This step was actually not necessary. Should I test it regardless?

Finally, if the pkg `py37-scikit-learn` package is bumped to version 0.22 should I test it instead of building from the Ports Collection as well?

Thanks a lot for your assistance :).
Comment 6 Kubilay Kocak freebsd_committer freebsd_triage 2019-12-20 01:42:54 UTC
(In reply to Andy Mender from comment #5)

Thank you Andy :)

"I can confirm that it works as intended and does not throw the error mentioned before."

That's good, we can set this issue to depend on bug 238718 as the update includes something that presumably resolves the bug, unless this is incorrect (please let us know).

"This step was actually not necessary. Should I test it regardless?

Yes please, because conversely, if the update doesn't (actually, either way), this will highlight LDFLAGS=-lblas removal in previous revisions are a contributor, if not cause of the issue.

As such, we could resolve the issue with reverting that removal, separately from any version update, which we prefer, as bugfixes can go to the quarterly branches, whereas there's less of an appetite to merge version updates to quarterly.

"should I test it instead of building from the Ports Collection as well?
"

Since packages are just precompiled ports, a (runtime) verified ports build also verifies the package runtime that will come once the port is updated
Comment 7 Andy Mender 2019-12-20 10:22:58 UTC
(In reply to Kubilay Kocak from comment #6)

> That's good, we can set this issue to depend on bug 238718 as the update includes something that presumably resolves the bug, unless this is incorrect (please let us know).

I think it makes sense to make my bug report depend on bug 238718. Since I tested py-scikit-learn version 0.20 in both Python2.7 and Python3.6, and saw the error there, it seems the update to 0.22 fixed the issue.

> Yes please, because conversely, if the update doesn't (actually, either way), this will highlight LDFLAGS=-lblas removal in previous revisions are a contributor, if not cause of the issue.

I got a little lost here. Should I then try version 0.20 with the explicit `LDFLAGS=-lblas` flag? Because if version 0.22 solved the problem, directly linking against `cblas` that way won't change anything or will it?

I'm clear on the rest of the items you listed :).
Comment 8 Kubilay Kocak freebsd_committer freebsd_triage 2019-12-20 10:27:28 UTC
(In reply to Andy Mender from comment #7)

"Should I then try version 0.20 with the explicit `LDFLAGS=-lblas` flag? "

Yes, but I'm not necessarily suggesting it for commit. 

To clarify my comment 6:

- science/py-scikit-learn is 0.20.* (and broken) in latest (head) *and* quarterly branches
- quarterly branches lean towards *not* merging 'version' updates
- if LDFLAGS=-lblas fixes the issue *as reported*, we can merge *just* that change to head, then merge it to quarterly, before we update this port to 0.22.*

But in order to do so, we need confirmation that LDFLAGS removal (in previous commits) was the actual cause of the error, and putting it back resolves the issue, without any other changes
Comment 9 Andy Mender 2019-12-20 11:31:12 UTC
(In reply to Kubilay Kocak from comment #8)
Now everything's clear, thanks! :)

I will do as suggested and post the results as soon as I can.
Comment 10 Andy Mender 2019-12-21 19:37:42 UTC
(In reply to Andy Mender from comment #9)

I built math/cblas, math/py-numpy and math/py-pandas for Python3.6 from the Ports Collection, and subsequently built science/py-scikit-learn with the suggested `LDFLAGS=lcblas` setting. On top of the previous error, I also got the below note from scikit-learn:
```___________________________________________________________________________
Contents of /usr/local/lib/python3.6/site-packages/sklearn/__check_build:
__init__.py               _check_build.so           setup.py
__pycache__
___________________________________________________________________________
It seems that scikit-learn has not been built correctly.

If you have installed scikit-learn from source, please do not forget
to build the package before using it: run `python setup.py install` or
`make` in the source directory.

If you have used an installer, please check that it is suited for your
Python version, your operating system and your platform.
```

Explicitly linking against cblas in the Makefile of science/py-scikit-learn-0.20.3 doesn't help. From what I saw, science/py-scikit-learn-0.22 no longer depends on cblas and it works without issues.

I think it's fair to bump the port to version 0.22.
Comment 11 Andy Mender 2019-12-21 21:29:06 UTC
(In reply to Andy Mender from comment #10)

My apologies! I just now realized `cblas` and `blas` are NOT the same thing.

I tested science/py-scikit-learn-0.20.3 again, this time linked against blas (not cblas!) via `LDFLAGS=-lblas`and it DID in fact work. I got the following warnings while running my SVM model, but it still generated the expected results:
```
/usr/local/lib/python3.6/site-packages/sklearn/utils/validation.py:595: DataConversionWarning: Data with input dtype int64 was converted to float64 by StandardScaler.
  warnings.warn(msg, DataConversionWarning)
/usr/local/lib/python3.6/site-packages/sklearn/utils/validation.py:595: DataConversionWarning: Data with input dtype int64 was converted to float64 by StandardScaler.
  warnings.warn(msg, DataConversionWarning)
/usr/local/lib/python3.6/site-packages/sklearn/utils/validation.py:595: DataConversionWarning: Data with input dtype int64 was converted to float64 by StandardScaler.
  warnings.warn(msg, DataConversionWarning)
```

I also suggest this change to the LIB_DEPENDS in the Makefile to make it depend on math/blas, instead of math/cblas like it did before:
`LIB_DEPENDS=	libblas.so.2:math/blas`
Comment 12 Kubilay Kocak freebsd_committer freebsd_triage 2019-12-27 06:09:15 UTC
(In reply to Andy Mender from comment #11)

So to confirm:

 - LDFLAGS=-lblas works with (fixes) 0.20.x
 - 0.22.x works without changes (as it is in the ports tree today)

Is that correct?

Further, the 0.22.x update appears to include some API changes and features (some declared as 'major' (?)) [1] so this issue is, then:

At the moment, if 0.22.x works in latest, only broken in the quarterly version (0.20.x) branch.

This issue is then (unfortunately), now, probably best resolved "Overcome By Events", given:

- We very rarely do direct to quarterly commits without a corresponding head commit, which is not required if 0.22 works

- A new quarterly branch will be cut soon (after December 31)

What should probably happen now not withstanding no other changes, is that a test suite and target should be added to the port to allow easily and comprehensively testing the runtime behaviour of the port prior to commits.

I'll leave that to the current (new) maintainer Wen

[1] https://scikit-learn.org/stable/whats_new/v0.22.html

^Triage: Assign to new committer Wen
Comment 13 Wen Heping freebsd_committer freebsd_triage 2020-03-15 23:38:16 UTC
Shall I close this PR since the current version in ports only works with python3 ?

wen
Comment 14 Kubilay Kocak freebsd_committer freebsd_triage 2020-03-16 06:26:00 UTC
(In reply to Wen Heping from comment #13)

This is a bug report for a Python 3.x build, and currently (apparently) depends on a scikit-learn update. This bug depends on bug 238718, which apparently works. 

So in order to consider this issue resolved, we'd need to get confirmation after a/the 0.22.0 update that Andy's (this bugs reporter) issue is resolved.

Comment 12 provides the summary so far.
Comment 15 Kubilay Kocak freebsd_committer freebsd_triage 2020-03-16 07:35:37 UTC
@Reporter (Andy) 

scikit-learn was updated to 0.22.0 via bug 238718

Could you please confirm whether or not the issue as reported here is still reproducible after updating your ports tree to the latest versions and testing 0.22.0, and if so, provide/reconfirm steps to reproduce
Comment 16 Andy Mender 2020-03-16 09:06:35 UTC
(In reply to Kubilay Kocak from comment #15)
science/py-scikit-learn version 0.22 works as intended. The PR can be closed. Huge apologies for the lack of replies for a longer while.