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

Removed plural forms of certain parameter names (fixes #531) #551

Merged
merged 5 commits into from Nov 29, 2016

Conversation

@heplesser
Copy link
Contributor

heplesser commented Nov 21, 2016

Removed plural forms of taus_* parameter names, removed duplicate and unused declarations and names from nest_names.

This fixes #531 .

I suggest @Silmathoron and @golosio as reviewers.

… unused declarations and names from nest_names.
@golosio
Copy link
Contributor

golosio commented Nov 22, 2016

@heplesser all the changes are fine for me.

Copy link
Member

Silmathoron left a comment

All plural forms are okay for me!
Further modifications involve removal of:

  • A (could not verify this)
  • as (could not check this)
  • dc (indeed not required anymore -- replaced by rate_)
  • dhaene_* (not required anymore)
  • distribution (defined twice)
  • eqX (not required anymore)
  • exc_conductance (logical removal, but potential problem with test_hh_cond_exp_traub.sli
  • I_ex/I_in (my bad, I forgot to remove these ones ^^")
  • I (okay, but ticket-466.sli should also be updated)
  • input_currents_* (OK, my bad again)
  • lin_* (ok)
  • ns (could not check this)
  • pot_spikes (ok)
  • ps (could not check this)
  • xs (ok)

Since everything compiles fine, I guess the modifications I could not check are fine, though.

@heplesser
Copy link
Contributor Author

heplesser commented Nov 25, 2016

@Silmathoron Thanks for your careful check. I checked all removed names by a project wide search, and indeed compilation would break if a name were missing.

  • I will change the Traub-model test to use g_exc and g_inh instead of the ..._conductance names.
  • I don't understand your comment about I and ticket-466.sli. Did you mean another ticket?
    I will now try to fix the merge conflicts. I fear we may be getting into a mess here ...
@Silmathoron
Copy link
Member

Silmathoron commented Nov 25, 2016

Ok, good!
For ticket-466.sli, unless I'm mistaken, mirollo_strogatz_ps does not exist anymore (which is why you could remove I...).
Yes, this looks rather bad :s good luck with that!

heplesser added 3 commits Nov 25, 2016
…fix-531-singular-names

# Conflicts:
#	nestkernel/nest_names.cpp
#	nestkernel/nest_names.h
@heplesser
Copy link
Contributor Author

heplesser commented Nov 25, 2016

@Silmathoron I merged, wasn't as bad as I thought. I decided to keep the Mirollo-Strogatz-related names in, I assume someone is tending the model somewhere and it might come back. Would you have another look and the approve (or not)?

Copy link
Member

Silmathoron left a comment

All good! 👍

@heplesser heplesser merged commit 20a164d into nest:master Nov 29, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@heplesser heplesser deleted the heplesser:fix-531-singular-names branch Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.