Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add decision tree and random forest #158

Closed
wants to merge 3 commits into from

Conversation

BMJHayward
Copy link

No description provided.

@BMJHayward
Copy link
Author

BMJHayward commented Mar 10, 2023

@ulises-jeremias added this PR as draft til I can work out a compiler issue when running ml/tree_test.v :
Any advice?

v -version V 0.3.3 32d0954
==== TEST OUTPUT ====

---- Testing... ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 FAIL   626.104 ms /home/brendan/code/vsl/ml/tree_test.v
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:25179: warning: assignment from incompatible pointer type
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:25187: warning: assignment from incompatible pointer type
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:25188: warning: assignment from incompatible pointer type
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:25189: warning: assignment from incompatible pointer type
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33190: warning: implicit declaration of function 'LAPACKE_dgesv'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33197: warning: implicit declaration of function 'LAPACKE_dgesvd'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33205: warning: implicit declaration of function 'LAPACKE_dgetrf'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33214: warning: implicit declaration of function 'LAPACKE_dgetri'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33223: warning: implicit declaration of function 'LAPACKE_dpotrf'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33246: warning: implicit declaration of function 'LAPACKE_dgeev'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:33254: warning: implicit declaration of function 'LAPACKE_dlange'
/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test.1911505891131190714.tmp.c:35274: error: '{' expected (got ";")
builder error: 
==================
C error. This should never happen.

This is a compiler bug, please report it using `v bug file.v`.

https://github.com/vlang/v/issues/new/choose

You can also use #help on Discord: https://discord.gg/vlang

-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Failed command 1:    '/home/brendan/code/v/v' -cg  -o '/tmp/v_1000/tsession_7fc53e46cb80_75632863592310/tree_test' '/home/brendan/code/vsl/ml/tree_test.v'
Summary for all V _test.v files: 1 failed, 1 total. Runtime: 626 ms, on 1 job.

==== END TEST OUTPUT ====

@ulises-jeremias
Copy link
Member

@BMJHayward hey! yes, it is because of missing dependencies. For now vsl.la and vsl.ml depends on LAPACKE :/

if you can send the output of v doctor here I can send you more clear steps on how to solve it for your system 👌🏻

probably the easiest way will be using docker

@BMJHayward
Copy link
Author

Thanks, I attempted to use the docker build with the vs code plugin, no luck as of yet. Here's the output of V doctor:

OS: linux, Linux Mint 20.1
Processor: 24 cpus, 64bit, little endian, AMD Ryzen 9 3900X 12-Core Processor
CC version: cc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0

getwd: /home/brendan/code/vsl
vmodules: /home/brendan/.vmodules
vroot: /home/brendan/code/v
vexe: /home/brendan/code/v/v
vexe mtime: 2023-03-10 06:05:03
is vroot writable: true
is vmodules writable: true
V full version: V 0.3.3 32d0954

Git version: git version 2.40.0
Git vroot status: weekly.2023.10-21-g32d09544 (49 commit(s) behind V master)
.git/config present: true
thirdparty/tcc status: thirdparty-linux-amd64 12f392c3

@ulises-jeremias
Copy link
Member

ulises-jeremias commented Mar 23, 2023

great! because you are on Ubuntu you can install the same dependencies we install on CI to use your own machine. You can run the following to fix this issue and any possible issue for the future 😅

sudo apt-get update && \
          sudo apt-get install --quiet -y --no-install-recommends \
            gfortran \
            libxi-dev \
            libxcursor-dev \
            mesa-common-dev \
            liblapacke-dev \
            libopenblas-dev \
            libgc-dev \
            libgl1-mesa-dev \
            libopenmpi-dev \
            libhdf5-dev \
            hdf5-tools \
            opencl-headers

Comment on lines +8 to +10
pub type Tree = Empty | Node

pub struct Empty {}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not up to date with recent V but some months ago this has been a huge anti-pattern.

IMHO this should be an optional - namely:

pub type Tree = Node | none

none guarantees in compile-time that you will never forget to check the variable of type Tree might be empty. It is also (much) faster in performance.

Or maybe just do not use any auxiliary/proxy type and use none directly:

[heap]
pub struct Node {
mut:
	feature   int
	threshold f64
	left      Node | none
	right     Node | none
	value     f64
}

(I do not know if this works but it should IMHO)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you! I agree 👌🏻 We can wait until the PR is ready for review before starting adding more comments and suggestions 😊

I'll assign you as a reviewer as soon as @BMJHayward says it is ready

@ulises-jeremias
Copy link
Member

closing this PR due to inactivity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants