30

I think it's a great project! But it could do with a few improvements: Neuron Type(1) Suppose we have a network of perceptrons that we'd like to use to learn to solve some problem. For example, the inputs to the network might be the raw pixel data from a scanned image of a signature. And we'd like the network to learn weights and biases so that the output ...


16

In addition to Toby Speight's remarks, I would add: Use constexpr for all constants I see you already used constexpr for image_code and label_code, but you didn't use it for the static member variables rows, cols and labels of MNISTObject. But those look like they are constants as well. You can just write: struct MNISTObject { ... static constexpr ...


14

Whitespace. It looks nice, and is important. Use it. There are many areas here where you could insert some whitespace, and then the code will magically become much easier to read. Here are a few areas where whitespace is needed. One blank line between the functions in bot. Some more blank lines in between blocks of code in the module-level, and in any ...


13

Genesis It's best practices to let statements like if, for, try ... be followed by a space to disinguish them from method invocations. protected String last = null; protected is pointless since Genesis is a final class and hence can't be extended. This variable holds the last what? Advancing in the code it's apparentlylastResponse. Let's rename it ...


11

Just a quick remark: the amount of horizontal scrolling doesn't make for very readable/maintainable code: private static String transform(String s) { //transform a string into something the program can read return s.toLowerCase().replace("?", "").replace(".", "").replace("!", "").replace(",", "").replace("_", "").replace("~", "").replace("`", "").replace("'"...


10

That's a very big list of includes! It might be a sign that you have functionality that should be split out to separate source files (e.g. filesystem read/write away from the core computation). Keeping the standard library includes alphabetical is a good choice - it allows very easy checking of whether a needed header is already included. This looks broken,...


9

I'm the author of core.matrix, so hopefully I can give you some tips from that perspective. If you want to improve performance, it's much better to use vectors in an optimised format throughout (vectorz-clj is a fine choice) rather than mixing in Clojure vectors everywhere. This saves the overhead of converting to/from Clojure vectors all the time, which is ...


8

Structure Nothing! The code seems fairly straightforward and clear, and I was able to understand what you’re doing even if I didn't study flask too much. So I don’t think there’s anything wrong with the structure. Code style One small nitpick: some of your lines are too long (>79 characters). From PEP8: Limit all lines to a maximum of 79 characters. ...


8

Just reviewing normalizeFeatures. Instead of a comment explaining what the function does, write a docstring. (Docstrings are available from the interactive interpreter via the help function.) The function operates on the global variable X. This makes the function inflexible (you can't use it for anything other than modifying the particular variable X), and ...


8

Looks plausible. The two biggest pieces of advice I have for you are: Format your code consistently and idiomatically! One easy way to do this is to use the clang-format tool on it. A more tedious, but rewarding, way is to study other people's code and try to emulate their style. For example, you should instinctively write vector<T>, not vector <T&...


8

Toby Speight and G. Sliepen gave excellent feedbacks from the programmer's perspective; A friend of mine gave some feedbacks from the machine learning researcher's perspective, as follows: User should be able to specify hyperparameters (lr, weight_decay, etc), or the code should include hyperparameter tuning Weight matrix initialization is wrong. It should ...


7

var distances = new double[trainNumber][]; for (var i = 0; i < trainNumber; i++) { distances[i] = new double[2]; // Will store both distance and index in here } This is a code smell. You shouldn't use a jagged double array to store an array of distances and indexes. Despite the comment, what you're doing is unclear, and it's very confusing to have a ...


7

Your code looks fine and is vectorized! You could've written a single line logistic cost function, but I believe your approach is more readable. Good job! I don't think there is much more optimizations that you can do related to the basic form of logistic regression. A possible addition however is to add regularization. This helps for the scenario of ...


7

Object orientation I don't like the Bot object in it's current state. You've just shoved all your code into it. I would rather see you isolate separate things into their own objects/functions. I think the Bot class should only deal with the machine learning part of the problem, i.e. take a string and return a response. It should not deal with terminal I/O ...


7

Use a linter The first thing I do when reviewing JavaScript code is run it through a linter. There several online linters, JSHint for example. JSHint will complain about ES6 features: 'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6'). To get around this, add this comment line at the top: // jshint esversion: 6 It's recommended ...


7

Disclaimer: I'm not familiar with the work you're doing, so I'll limit my comments to the structure of your code. Generally, I'd make the following changes: Split data and code Write more functions Don't repeat yourself Split data and code You have a lot of lines that contain both hardcoded information to display to the user and calculations that are ...


7

About OOP I tried to exploit OOP as much as I could, instead of using a procedural approach to write the algorithm. Although I believe that your approach was fine, using OOP for the sake of OOP is something I would rather warn against. There is a talk about this here. Comments def __init__(self, dataset, learning_rate, num_iterations): """ ...


7

I don't know much about machine learning, so I'll be reviewing your style. Also, this is my first review, I hope I do it right. Use Python 3 Judging by your print statements, you are currently using Python 2.x. Python 2 support will be dropped soon, and there is no reason to use it today (except if maintaining legacy code, which is not your case). More ...


6

1. Comments on your code There's no documentation! What do these functions do? How do I call them? If someone has to maintain this code in a couple of years' time, how will they know what to do? alpha should be a property of the class (and thus an optional argument to the constructor), not an optional argument to the __updateWeights method (where the user ...


6

You should definitely avoid computing distance from every training sample. That's the main cause of inefficiency. By using proper data structure, the search for nearest neighbours can be done in \$O(log(n))\$. Your code does that in \$O(n)\$, where \$n\$ is the number of samples. Technical improvements might give you 2-times speed-up, but this will give you ~...


6

Finding the average output Let's figure out what your average output should be. Your neural network has 3 inputs in the first layer, 2 nodes in the second layer, and one output. Each weight is randomized to a value from 0..1, so call it 0.5 on average. The inputs you use in the program are: 1, 0, 1. On each layer, you also have a "bias" input of ...


6

Quickly reading the wikipedia link you provided, it sounds like your implementation slightly differ from the theory, especially the initialization part. I’m not expert, however, and will not try to dig it further. Just going for a style review: Keep spaces consistent As it currently stand, your spaces around = or , are not consistent and can be quite ...


6

You can gain some efficiency by wrapping the writer in a BufferedWriter. If you are using java 7 or better you should use try-with-resources to auto close the writer, otherwise you should use a try-finally: try(BufferedWriter writer = new BufferedWriter(new FileWriter(outputFile))){ //the for loops } with try-finally: BufferedWriter writer = new ...


6

So, let's focus on your Node class at the top. I started off, without looking at anything else, and just swiftlinting it. You have a 55 line file with 20 violations. Fortunately, 19 of them are autocorrectable. Seven of the violations are for opening brace placement. In Swift, we prefer our opening brace to be on the same line rather than new line. ...


6

You can vectorize this at various points to apply arithmetic to the whole dataframe rather than row-by-row. def min_euclidean(df, options): """ Returns the index of the series in iterable options for which df - row has minimum Euclidean distance """ return pd.DataFrame(((df - series) ** 2).sum(axis=1, skipna=False) ...


6

#define WEIGHT_RANDOM_RANGE .1 Don't use #define to make your constants. Use constexpr. In your CSV parser: scalar value = scalar(std::stof(line.substr(0, found))); line = line.substr(found + 1, line.size()); That's grossly inefficient. The string::substr creates a new string and copies data. Then you shorten the input by using substr again and ...


5

Let's start with the obvious remarks about whitespace: Whitespace is important in Python. You got trailing whitespaces all over the place and you use an indentation of 2 spaces where 4 is prescribed by the official PEP8 Style Guide. When talking about sticking to best practices in Python, starting with PEP8 is a good idea. There's a lot more violations ...


5

You can avoid appending to vectors (which can cause re-allocation of space and can considerably slow things down in principle; though in your case of only a length 10 vector that shouldn't be noticeable) if you allocate them to the needed size initially and then assign within them. result <- vector("numeric", 10) # or even: result <- numeric(10) n <...


5

Disclaimer: I know practically nothing about ML or neural networks. The big problem with this program is readability. There are no docstrings or comments, so even somebody who knows about ML would have difficulty using this. for example, what arguments should I pass to the ConvPoolLayer constructor? What does reLU(z) represent? And so on. Particularly when ...


5

My biggest piece of advice would be to replace freqSet = defaultdict(int) with a Counter. Counters are a datatype designed to do exactly what you are doing with defaultdicts, and they have some specialized methods. for item in itemSet: for transaction in transactionList: if item.issubset(transaction): ...


Only top voted, non community-wiki answers of a minimum length are eligible