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

Make non-thread-safe warnings more specific #1327

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

JCGoran
Copy link
Contributor

@JCGoran JCGoran commented Jun 18, 2024

The warning MOD file uses non-thread safe constructs of NMODL does not really tell me which ones are not thread-safe, so this PR makes it more specific and now outputs a warning with about the ones which are not safe. More of a UX improvement than anything else.

@JCGoran JCGoran requested review from pramodk and 1uc June 18, 2024 11:37
bbpadministrator pushed a commit to BlueBrain/nmodl-references that referenced this pull request Jun 18, 2024
@bbpbuildbot
Copy link
Collaborator

Logfiles from GitLab pipeline #217627 (:no_entry:) have been uploaded here!

Status and direct links:

Copy link
Collaborator

@1uc 1uc left a comment

Choose a reason for hiding this comment

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

Seems error prone and vectorize is apparently obsolete, we're working towards removing it.

@pramodk
Copy link
Contributor

pramodk commented Jun 19, 2024

@JCGoran: just to expand a bit Luc’s comment: In the context of neuron, we will need to support code generation for all cases i.e. everything that nocmodl supports today. As part of the PR like neuronsimulator/nrn#2906, we are removing/refactoring parts related to VECTORIZE. Hence this wouldn’t be needed.

@1uc
Copy link
Collaborator

1uc commented Jun 19, 2024

I'm sorry, I confused myself. We're getting rid of vectorize by renaming it thread-safe and sometimes genuinely removing it. The problem is that vectorize refers to both putting data into vectors (which we always do nowadays); and being thread-safe (which MOD file may or may not be). These instances would likely just become info.thread_safe.

Personally, I'd do it differently. Keep the catch all as a warning (because users might want to know that something "is up") and make the new log messages info or debug (because the details are only important while debugging).

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.

4 participants