Skip to content

Remove how-to example for support vector machine #6201

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

Merged
merged 1 commit into from
Jun 23, 2022

Conversation

epa095
Copy link
Contributor

@epa095 epa095 commented Jun 15, 2022

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?
  • Delete "how to use a library" example.

This PR deletes an "algorithm" which purely calls an external library for the computation, and only functions as a how-to. The contributing guide states:

Algorithms in this repo should not be how-to examples for existing Python packages. Instead, they should perform internal calculations or manipulations to convert input values into different output values. Those calculations or manipulations can use data types, classes, or functions of existing Python packages but each algorithm in this repo should add unique value.

The deleted files does nothing of the support-vector machine work, it only shows how to use SVM's from scikit-learn.

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • All new Python files are placed inside an existing directory.
  • All filenames are in all lowercase characters with no spaces or dashes.
  • All functions and variable names follow Python naming conventions.
  • All function parameters and return values are annotated with Python type hints.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

@ghost ghost added enhancement This PR modified some existing files awaiting reviews This PR is ready to be reviewed labels Jun 15, 2022
@@ -1,58 +0,0 @@
from sklearn import svm
Copy link
Member

Choose a reason for hiding this comment

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

Why do we want to remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One reason is that it is in conflict with the contribution guide, as quoted above.

Another is that the value of this repo is really the algorithms, and that you can read the code and learn how the algorithm works. An example is the linear regression file, which actually teaches you how linear regression works. As it is now this repo consists of a mix of actuall algorithm-implementations, which you can read and learn from, and pure usage-examples, which provide much less value. It clutters up the repo, and if we removed them then maybe we could get submissions with the actuall algorithms instead.

Copy link
Member

Choose a reason for hiding this comment

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

Ok I see your point - and it's totally correct here in this case. Would you be able to replace this with the actual SVM algorithm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately not, the reason I browsed this file was because I wanted to see how SVM's works :-p But hopefully someone competent will contribute!

Copy link
Member

Choose a reason for hiding this comment

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

After some thoughts, I think we need to discuss this - whether "how-to" examples are acceptable for "machine learning"-type of algorithms. Those algorithms usually consist of building blocks - like SVM, cost functions, neurons or CNN layers - and the use of those blocks might constitute an "algorithm" itself. However, I would consider for example add(x, y) to add two numbers a "how-to" example and removing it should make more sense: there is a + operator!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we don't start at the nand-gate, there is always a lower level ;-) E.g. in the linear regression example, you need to know what np.dot does, what transpose is, etc. I guess deciding exactly the right level is a bit like editing a text book, and you need to have a "feel" for how "far down" you need to explain.

I don't know if it is possible to agree on some "rule of thumb" for what to keep. But for me I imagine a situation where I let a smart student read the code for some algorithm, and afterwards explain to me how that algorithm works. For the linear regression case I can imagine a good student would be able to explain linear regression the algorithm after having read the implementation here. For SVM she would not be able to give any better explanation than "SVM's work by importing scikit-learn".

BTW, it is definitely possible to imagine "a machine learning project" as an algorithm here, which uses e.g. SVM, cross validation, train/test/validate etc. But that would then explain "the machine learning process", and should not "pretend" to explain SVM.

Copy link
Member

Choose a reason for hiding this comment

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

I file an issue #6216 - I'd merge this first because it really doesn't show detailed implementation, but the next steps might be to figure out which files to replace because we might want to keep those contents available

poyea
poyea previously approved these changes Jun 22, 2022
Copy link
Member

@poyea poyea left a comment

Choose a reason for hiding this comment

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

Thank you for your pull request!🤩

@ghost ghost removed the awaiting reviews This PR is ready to be reviewed label Jun 22, 2022
@poyea
Copy link
Member

poyea commented Jun 22, 2022

There might be other similar files that do similar "how-to-use" thing - so it might be worth to list them in an issue so that we can replace them in the future.

Copy link
Member

@poyea poyea left a comment

Choose a reason for hiding this comment

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

We may need to change the guidelines, as we might change the definition of "how-to" examples

@poyea poyea merged commit 4a51244 into TheAlgorithms:master Jun 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This PR modified some existing files help wanted question
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants