2
\$\begingroup\$

I wrote a simple NN text classifier to help me quickly sort through the new daily submissions to the arXiv. It

  1. downloads the new submissions, processings their titles and abstracts,
  2. trains a NN on some manually-labeled data,
  3. uses the trained model to classify the new submissions, and
  4. presents the results allowing the user to either accept/reject/re-classify the submissions.

The implementation is pretty straightforward and can be found here: https://github.com/Gabriel-p/arXivNN. The two weaker spots I believe are:

  1. The soup parser which I'm sure could be done better
import re

from bs4 import BeautifulSoup as BS
import requests


    categ = "astro-ph"
    subcategs = ("astro-ph.GA", "astro-ph.IM")
    url = "http://arxiv.org/list/" + categ + "/new"
    html = requests.get(url)
    soup = BS(html.content, features="xml")

    # Store urls for later
    dt_tags = soup.find_all('dt')
    all_urls = [_.find_all('a')[1].get('id') for _ in dt_tags]
    all_urls = ["https://arxiv.org/abs/" + _ for _ in all_urls]

    # Extract titles and abstracts, only for the matching sub-categories
    dd_tags = soup.find_all('dd')
    articles = []
    for i, dd_element in enumerate(dd_tags):
        subjects = dd_element.find(class_='list-subjects').text
        subcategs_new = extract_text_in_parentheses(subjects)
        # Check if submission fits any sub-category
        if any(element in subcategs_new for element in subcategs):
            title = dd_element.find(class_='list-title mathjax').text
            title = title.split('\n')[1].strip()
            abstract = dd_element.find_all(class_='mathjax')[-1].text.strip()
            articles.append([title, abstract, all_urls[i]])


def extract_text_in_parentheses(text):
    pattern = r"\((.*?)\)"
    matches = re.findall(pattern, text)
    matches = [_.lower() for _ in matches]
    return matches
  1. The NN itself for which I basically just followed Claude's instructions
    preprocessed_texts = preprocess_text(texts)

    # Convert to TF-IDF representation
    vectorizer = TfidfVectorizer(max_features=max_features)
    tfidf_matrix = vectorizer.fit_transform(preprocessed_texts)
    X = tfidf_matrix.toarray()

    # Convert labels to categorical
    y = to_categorical(np.array(labels) - 1, num_classes=num_classes)

    # Split the data into training and testing sets
    X_train, X_test, y_train, y_test = train_test_split(
        X, y, test_size=0.2, random_state=42)

    # Build the neural network model
    model = Sequential([
        Input(shape=(max_features,)),
        Dense(64, activation='relu'),
        Dense(32, activation='relu'),
        Dense(16, activation='relu'),
        Dense(num_classes, activation='softmax')
    ])

    # Compile the model
    model.compile(
        optimizer='adam', loss='categorical_crossentropy', metrics=['accuracy']
    )

    # Train the model
    model.fit(
        X_train, y_train, epochs=100, batch_size=32, validation_split=0.2, verbose=verb
    )

    # Evaluate the model
    loss, accuracy = model.evaluate(X_test, y_test, verbose=verb)
    print(f"Test accuracy: {accuracy:.4f}")

In my tests I get a ~0.7-0.75 accuracy which is not bad I believe, but maybe it can be improved?

\$\endgroup\$
2
  • \$\begingroup\$ Welcome to Code Review. You supplied zero lines of code emedded in your question. Please offer enough source code so someone will be able to actually review it. \$\endgroup\$
    – J_H
    Commented Jul 17 at 12:49
  • \$\begingroup\$ Ok, I thought linking to the full repo was cleaner. I'll update the question \$\endgroup\$
    – Gabriel
    Commented Jul 17 at 13:11

1 Answer 1

3
\$\begingroup\$

deps

Your repo lacks {environment.yml, requirements.txt}, and the ReadMe offers no version advice. I used keras 3.4.1 and tensorflow 2.16.2.

annotations

Telling us the subcategories arrive in a tuple is nice, but prefer to be more specific: tuple[str, str]

Consider constructing url with an f-string.

wrong name

This is the wrong identifier:

    html = requests.get(url)

Please call it response, resp, even r, something like that. It contains HTML .content, but also metadata such as a status code. On which topic, right after the .get() it is usual to verify 200, to improve the quality of the diagnostic in case of network failure:

    response = requests.get(url)
    response.raise_for_status()

anonymous loop variables

    subcategs = [_.lower() for _ in subcategs]
    ...
    all_urls = [_.find_all('a')[1].get('id') for _ in dt_tags]
    all_urls = ["https://arxiv.org/abs/" + _ for _ in all_urls]

I get it, you're saying "I don't care about that temp var." But please use the conventional
for tag in dt_tags and
for url in all_urls so it will read nicely.

That first one would most naturally be expressed as
list(map(str.lower, subcategs))

missing punkt

Halting with a fatal error is pretty OK, as the library's diagnostic is quite helpful:

  Resource punkt not found.
  Please use the NLTK Downloader to obtain the resource:
  >>> import nltk
  >>> nltk.download('punkt')

But it would be nicer if you wrote a try / except handler which anticipates this issue and goes ahead with doing the download if it turns out to be necessary.

To its credit, the docstring quite thoroughly observes that "the necessary NLTK data" must first be downloaded. But why should caller need to know or care about such details? I'm suggesting that this function could sensibly shoulder such burden.

subsection comments

    # Store urls for later
    ...
    # Extract titles and abstracts, only for the matching sub-categories
    ...
        # Check if submission fits any sub-category

These are helpful comments, and I thank you for them. But if you feel the need to break out sections, that suggests maybe you should break out the occasional helper function. Not on every one of them, but they are kind of piling up here.

Let's review the docstring which explains this code's responsibility.

def get_arxiv_new(categ, subcategs):
    """
    Download recent submissions from arXiv for the given category. Keep only those
    that belong to any of the sub-categories selected.
    """

Beautiful docstring, keep writing those. But notice we needed two sentences, to describe the two responsibilities -- we download and we filter. Now, I'm not suggesting to change what this function does, as it seems quite sensible, a natural unit of work. But I would like to see the filtering aspect broken out. For one thing, that would make the filtering logic much easier to unit test.

Also, your pickle comments suggest that you'd like to become acquainted with the requests-cache module.

Your CSS selectors are very clear, and generally the soup filtering is some nice code. It appears you're being aided here by the arXiv's generated HTML being fairly clean and easy-to-parse.

The non-greedy ? regex is nice.

redundant docstrings

Generally, your docstrings are very nice; you should keep writing them.

There is some redundancy, so you might consider writing fewer details in the English prose. When comments and code say different things, we complain that "comments lie". Here is a small example:

    categ: str = "astro-ph",
    ...
    Parameters:
    -----------
    categ : str, optional

You didn't tell mypy that it is Optional (with str | None), but you're inviting human engineers to pass in None. Or perhaps the imprecise English word "optional" denotes "keyword defaulted" here, which was already obvious from viewing the signature.

    subcategs: tuple = ("astro-ph.GA", "astro-ph.IM"),
    ...
    subcategs : tuple of str, optional
        Subcategories within the main category to consider (default is
        ("astro-ph.GA", "astro-ph.IM")).

Please tell mypy we have tuple[str, str]. And copy-n-pasting those values makes me nervous that a maintenance engineer will change one and not the other.

In a language analysis program, the identifier verb is too terse, as though it might appear next to noun. Prefer verbose. Also, it's slightly surprising that it's not bool, and we're given little guidance about sensible values to try.

doctests

Some of your docstrings include examples, which is terrific!

def predict_label( ... ):
    """
    ...
    Example:
    >>> df = pd.DataFrame( ... )
    >>> articles = [("Title 1", ...), ("Title 2", ...)]
    >>> predict_label(df, articles, my_vectorizer, my_model, verb)
    """

But alas, when we use $ python -m doctest *.py to measure how truthful the comments are, it turns out that third line is invalid, we haven't defined such a vectorizer.

Similarly for train_NN():

    Example:
    >>> df = pd.DataFrame({'abstract': ['text1', 'text2'], 'class': [1, 2]})
    >>> vectorizer, model = train_NN(num_classes=2, max_features=1000, df=df, verb=0)

The shape of df is incorrect; the lack of a df.text column will raise KeyError.

wrong max_features

This doesn't seem to be correct:

    vectorizer, model = train_NN(num_classes, max_features, df_class, verb)

When I use the ReadMe's suggested CSV input, I obtain

ValueError: Exception encountered when calling Sequential.call().
Input 0 of layer "dense" is incompatible with the layer: expected axis -1 of input shape to have value 1000, but received input with shape (None, 184)

Arguments received by Sequential.call():
  • inputs=tf.Tensor(shape=(None, 184), dtype=float32)
  • training=True
  • mask=None

Reducing max features turns that bad behavior into an infinite hang. Consider adding a progress bar if there is quite a lot of compute to do.

approach

The train_NN() function looks standard enough. (PEP 8 asks that you downcase the identifier.)

It's unclear what embedding you're shooting for, and why the proposed neural architecture is a good fit for that. I'm especially concerned about having sufficient number of labeled examples.

Consider starting out with a latent Dirichlet allocation model, either the one in sklearn or perhaps Gensim, which has nicer scaling properties. Rather than prompting a human operator for small integer labels, you may find that auto-extracted LDA topics are sufficient for your purposes. Armed with such output, you're in a better position to throw lots of training data at whatever NN you wind up settling upon.

\$\endgroup\$
1
  • \$\begingroup\$ Great comments, thank you very much! Regarding your doubts about the NN itself, I just used what Claude AI recommended. In fact almost all of the docstrings are written by Claude. I'll check what you say about LDA and will fix the issue with max_features making the code catch that mismatch and change that argument if required \$\endgroup\$
    – Gabriel
    Commented Jul 17 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.