Complex compartments #54
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Use divided compartments to determine FVM coefficients. * Pick correct control volume in FVM from sgement position (avoids off-by-half error.) * Add colour override functionality to tsplot: `--colour` option. * Add const accessor for cell soma. * Source formatting, comments in `math.hpp` * Fix `range_view`: was using incorrectly named type trait. * Add unit test for `range_view`. * Allow points of discontinuity to be omitted from L-infinity norm calculations. * Add `-d, --min-dt` option to `validate.exe` to control time step in validation convergence tests. * Add validation test: confirm divided compartment policy does not effect results on simple frustrum dendrites. * Change default max compartments on validation tests to 100 (ad hoc observed convergence limit at dt circa 0.001 ms; finder spatial division would required much finer dt.) * Make NEURON validation data generation scripts use CVODE by default, and with `secondorder=2` when non-zero `dt` is given.
bcumming
requested changes
Oct 31, 2016
| @@ -233,8 +247,8 @@ class fvm_multicell { | |||
| //////////////////////////////// Implementation //////////////////////////////// | |||
| //////////////////////////////////////////////////////////////////////////////// | |||
|
|
|||
There was a problem hiding this comment.
The DivPolicy is used in one location. I wonder if it needs to be its own template parameter? Maybe when we construct a cell we can pass the policy as a parameter to the constructor?
There was a problem hiding this comment.
Yup, let's bake in the default for now and add it back as part of a backend policy class.
| template <typename T, typename I> | ||
| void fvm_multicell<T, I>::compute_cv_area_unnormalized_capacitance( | ||
| template <typename T, typename I, typename DivPolicy> | ||
| void fvm_multicell<T, I, DivPolicy>::compute_cv_area_unnormalized_capacitance( | ||
| std::pair<size_type, size_type> comp_ival, | ||
| const segment* seg, | ||
| index_type &parent) |
There was a problem hiding this comment.
I don't think you need the using util::left/right statements below (I can't seem to leave a comment on the lines themselves)
| @@ -21,6 +21,11 @@ namespace mc { | |||
|
|
|||
| double linf_distance(const trace_data& u, const trace_data& ref); | |||
|
|
|||
| // Compute linf distance as above, excluding samples near | |||
There was a problem hiding this comment.
The comments above function definitions is inconsistent. in trace_analysis.cpp
/*
* Description of function
*/
Then here we are using single // comments.
Elsewhere we have used /// to indicate a descriptive comment above functions (and their prototypes).
I certainly prefer that to the 'noisy' style in trace_analysis.cpp.
There was a problem hiding this comment.
Sure. I don't have a strong preference one way or the other.
* Use only `div_compartment_integrator` for compartmentalization in `fvm_multicell`. The policy will later be moved to a backend policy class. * For now, disable validation tests that test different division policies (see above). * Tweak comments and remove redundant `using`, following comments on PR#54.
bcumming
approved these changes
Oct 31, 2016
noraabiakar
pushed a commit
to noraabiakar/arbor
that referenced
this pull request
Nov 26, 2019
Implements issue arbor-sim#54. * Support augmenting simulator names with ':tag' suffixes for configuring simulator behaviour in validation tests. * Split out common support code in Arbor validation model implementations (command line argument handling; netcdf exception wrapper). * Add new exit code representing failure for a model implementation to understand a requested tag. * Make command line arguments in model implementations clearer (output given by '-o', tags by '--tag' etc.) * Extend model_common.sh to help with simulator tag handling. * Add tag 'binevents' for existing Arbor models, 'firstorder' for NEURON models. * Update documentation to suit.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
--colouroption.math.hpprange_view: was using incorrectly named type trait.range_view.-d, --min-dtoption tovalidate.exeto control time step in validation convergence tests.secondorder=2when non-zerodtis given.