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.

Sign up for GitHub

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

Jump to bottom

Review timeseries #144

Merged
merged 70 commits into from
Feb 15, 2018
Merged

Review timeseries #144

merged 70 commits into from
Feb 15, 2018

Conversation

stuart-knock
Copy link
Member

@stuart-knock stuart-knock commented Jan 30, 2018 •
edited
Loading

#137 #138 #140
Reviews Timeseries, standardising some of the notation and class naming. Also adds a simple Sine Timeseries and a PulseSigmoid Timeseries.

Example output of PulseSigmoid from the following .conf, comparing with PulseRect

Time: 15 Deltat: 0.0001
Nodes: 144

    Connection matrix:
From:  1 
To 1:  0

Population 1: Stimulation
Length: 0.5
 Stimulus: Superimpose: 2
 Stimulus: PulseRect - Onset: 2 Node: 72 Amplitude: 4.2 Width: 1.0 Period: 3 Pulses: 3
 Stimulus: PulseSigmoid - Onset: 2 Node: 42 Amplitude: 4.2 Width: 1.0 Period: 3 Pulses: 3

Output: Node: All Start: 0 Interval: 0.001 
Population: 1
Dendrite:  
Propagator:
Coupling:

pulsesigmoid_pulserect

There are a few remaining issue noticed during the process, added two new issues:
#143 #142 #145

@stuart-knock
Adds comments, expands documentation, minor clean-up.
e19ad7b
@stuart-knock
Reviews Const. Adds explicit links to TIMESERIES functions in file doc.
4b1c943
@stuart-knock
Reviews Pulse.
7b1413a
@stuart-knock
Uses different variables for different input parameters BrainDynamics…
915c8dd 
…USYD#138.
@stuart-knock
Replaces explicit loops with vector's assign method. Adds TODO.
6ea28ae
@stuart-knock
Bits and pieces while reviewing.
912980c 
  + Reserve memory for vector before loop (performance).
  + Replaces two step clear(), resize() with equivalent assign().
  + Uses assign() in place of explicit loop.
  + Adds more comments.
  + Adds more doc.
  + Adds more TODOs.
@stuart-knock
Minor clean-up, reorder operations, in init. Adds comments.
a9a70f4
@stuart-knock
Converts a few more Node indices from double to size_type.
5bc3a69
@stuart-knock
Renames TIMESERIES::Pulse to TIMESERIES::PulseRect.
a3cf3bd
@stuart-knock
Renames TIMESERIES::Sine() to TIMESERIES::PulseSine().
65efdf7
@stuart-knock
Moves PulseSine to group Pulse* timeseries. Adds doc.
74d2aa7
@stuart-knock
Removes redundant duration declaration.
c61f4ee
@stuart-knock
Uses "Amplitude" consistently as the keyword for amplitudes.
7f712f9
@stuart-knock
Adds a simple Sine Timeseries, two args: Amplitude; Frequency.
5beaf67
@stuart-knock
Adds a sigmoidal pulse train to TIMESERIES.
e7677fd 
  Working, but needs clean-up.
@stuart-knock
Merge branch 'review-timeseries' of https://github.com/stuart-knock/n…
84f2424 
…eurofield into review-timeseries
@stuart-knock
Fixes doc.
e7c9370
@stuart-knock
Fixes edge-case and cleans-up PulseSigmoid.
94459ee 
  + Moves onset to be a member variable of Timeseries.
  + Moves PiOnSqrt3 calculation to compile time.
  + Adjusts duration properly to account for resetting t.
  + Fixes variable types.
  + Expands comments.
  + Removes completed TODO.
@stuart-knock
Fixes PiOnSqrt3 definition for clang.
d89de6b
@stuart-knock
Removes redundant "series" initialiser.
fbdd649
@stuart-knock
Fixes variable initialisation.
2c299fd
@stuart-knock
Moves larger remaining TODOs to GitHub issues, BrainDynamicsUSYD#142, B…
70ca4dd 
…rainDynamicsUSYD#143.
@stuart-knock
Fixes constant name.
6ab473b
@stuart-knock
Merge branch 'master' into review-timeseries
634291b 
  Pulls in initial renaming from neurofield to nftsim.
@pausz pausz added the sci Identifies features that are of scientific interest. label Feb 5, 2018
@pausz pausz added this to the Mark II milestone Feb 5, 2018
@pausz
Copy link
Contributor

pausz commented Feb 5, 2018

Reproduced the example:
screenshot from 2018-02-05 13-45-39

pausz
pausz requested changes Feb 5, 2018
View reviewed changes
Copy link
Contributor

@pausz pausz left a comment

Choose a reason for hiding this comment

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

Before merging this pull request:

  1. Add examples to the documentation of
    a) PulseRect + PulseSigmoid using superimposition
    b) PulseSine
  2. Stage and commit updated NFTsim.pdf

@pausz pausz mentioned this pull request Feb 5, 2018
Closed
@stuart-knock
Copy link
Member Author

@pausz requested changes addressed by cc0f936
The two other recent commits (e467241 and 708ea8b) fix issues encountered while addressing your requested changes.

@pausz pausz self-assigned this Feb 15, 2018
@pausz
Copy link
Contributor

pausz commented Feb 15, 2018 •
edited
Loading

Check the Onset time of the PulseSigmoid and PulseRect:
screenshot from 2018-02-15 13-45-14

Check that a Rectangular pulse of width = time step:
screenshot from 2018-02-15 14-39-03

Before e467241 the shape of the pulse would have been a triangle.

See issue about the amplitude of PulseSigmoid as the width decreases #156

@pausz
Copy link
Contributor

pausz commented Feb 15, 2018

@stuart-knock Could you add and commit the images for the user manual please?

@stuart-knock
Copy link
Member Author

@pausz image files added with (91cf63e), apologies, I just forgot.

@pausz pausz merged commit 221cd20 into BrainDynamicsUSYD:master Feb 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pausz pausz pausz approved these changes

Assignees

@pausz pausz

Labels
enhancement new-feature sci Identifies features that are of scientific interest.
Projects
None yet
Milestone
Paper Publication
Development

Successfully merging this pull request may close these issues.
2 participants
@stuart-knock @pausz