search

fzalkow / cl-mlep

cl-mlep is a Common Lisp Machine Learning library for Educational Purposes.
MIT License
34 stars 6 forks source link

Update cl-num-utils #4

Closed Symbolics closed 1 year ago

Symbolics commented 1 year ago

Hello Frank,

I'm glad to have found this library; I've been looking for some machine learning code in Common Lisp that doesn't have a restrictive license, and found this because I'm the new maintainer of LLA, and when updating it in Quicklisp discovered some breakage.

cl-num-utils has been updated and renamed num-utils, and that's what now LLA uses. Here, when used together, there is a package name clash because LLA and cl-num-utils aren't in sync.

The fix is to change cl-num-utils -> num-utils in the mlep-add ASDF file.

fzalkow commented 1 year ago

Hi, thanks for raising this issue! I haven't used Lisp for years, so please refresh my background if I miss something.

After updating the Quicklisp client (ql:update-client) to the most recent version 2021-02-13, (ql:quickload :num-utils) throws an error, but (ql:quickload :cl-num-utils) succeeds. This indicates that cl-num-utils is still the right name for the lib. Am I wrong?

Symbolics commented 1 year ago

It sounds like you need to update the software distributions list in addition to the client. Try (ql:update-dist "quicklisp")

fzalkow commented 1 year ago

Thanks; that did the trick! I replaced cl-num-utils with num-utils (e6a115f). This should resolve this issue, right?

Symbolics commented 1 year ago

Yes, it should. Does cl-mlep have a test suite we can try?

fzalkow commented 1 year ago

We have, unfortunately, no test suite, but we can try to replicate the usage examples. When loading load-with-add.lisp, I get the following error:

"XREAL" is already a nickname for "CL-NUM-UTILS.EXTENDED-REAL".

I guess this is due to the lla package, which still depends on cl-num-utils instead of num-utils, which makes our update inconsistent with lla. https://github.com/tpapp/lla/blob/master/lla.asd#L17

Symbolics commented 1 year ago

Yes, that's exactly correct. I'm in the process of getting the new LLA into Quicklisp. Now that cl-mlep is 'ready', I'll go back and get that process moving. Once Quicklisp's LLA is the new one, then all should work. I'll report back here once that's done.

For reference, that is quicklisp/quicklisp-projects#2276

fzalkow commented 1 year ago

Great, thanks for your efforts! 👍

fzalkow commented 1 year ago

@Symbolics, can we close this issue?

snunez1 commented 1 year ago

Yes, let's do that. I wasn't able to test, but the new LLA seems to be functionally equivalent.

fzalkow commented 1 year ago

I verified that the PCA example from the documentation (where LLA is used) is working after updating quicklip, with a minor additional commit I have done now. Thanks!