Skip to content

multi gridding tests#2260

Merged
ramcdougal merged 18 commits into
neuronsimulator:masterfrom
TracyL5982:master
Mar 21, 2023
Merged

multi gridding tests#2260
ramcdougal merged 18 commits into
neuronsimulator:masterfrom
TracyL5982:master

Conversation

@TracyL5982

Copy link
Copy Markdown
Contributor

No description provided.

@ramcdougal ramcdougal self-assigned this Mar 12, 2023
@codecov

codecov Bot commented Mar 14, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2260 (3ee58c8) into master (f838453) will increase coverage by 1.64%.
The diff coverage is 90.90%.

@@            Coverage Diff             @@
##           master    #2260      +/-   ##
==========================================
+ Coverage   57.25%   58.89%   +1.64%     
==========================================
  Files         619      619              
  Lines      121571   121607      +36     
==========================================
+ Hits        69601    71620    +2019     
+ Misses      51970    49987    -1983     
Impacted Files Coverage Δ
share/lib/python/neuron/rxd/rxd.py 84.16% <90.90%> (+13.78%) ⬆️

... and 32 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ramcdougal ramcdougal changed the title multi gridding tests multi gridding tests [wip] Mar 20, 2023
@ramcdougal ramcdougal added wip rxd reaction-diffusion labels Mar 20, 2023
@ramcdougal ramcdougal changed the title multi gridding tests [wip] multi gridding tests Mar 20, 2023
@ramcdougal ramcdougal removed the wip label Mar 20, 2023
@ramcdougal ramcdougal requested a review from adamjhn March 20, 2023 20:32
@ramcdougal

Copy link
Copy Markdown
Member

I'm having some trouble convincing myself that the new tests are being run... @adamjhn do you see, e.g. test_multigridding_allowed being tested anywhere?

@alexsavulescu

Copy link
Copy Markdown
Member

I'm having some trouble convincing myself that the new tests are being run... @adamjhn do you see, e.g. test_multigridding_allowed being tested anywhere?

https://app.codecov.io/gh/neuronsimulator/nrn/pull/2260

@adamjhn adamjhn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes look good, I just have a few suggestions about the tests;
Looks like test_multi_gridding_1.py, test_multi_gridding_2.py and test_multi_gridding_mix.py are useful for debugging, but maybe not for CI?
Maybe reduce the size (larger dx or smaller L) and get rid of one or both of the test_multi_gridding_1.py or test_multi_gridding_2.py, (which are more a test of rxd.Rate for 3d models than multigridding).

Comment thread test/rxd/3d/test_multigridding_allowed.py Outdated
Comment thread test/rxd/3d/test_multigridding_allowed.py
Comment thread test/rxd/3d/test_multi_gridding_mix.py
Comment thread test/rxd/3d/test_multigridding_allowed.py Outdated
@ramcdougal ramcdougal merged commit a8f6e1f into neuronsimulator:master Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rxd reaction-diffusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants