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

Optional first arg for Vector.record can be an ARTIFICIAL_CELL #2858

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nrnhines
Copy link
Member

@nrnhines nrnhines commented Apr 25, 2024

The existing logic of the if statement that generated the error message "Optional first arg is not a POINT_PROCESS" was incorrect.

[ ] add test
[ ] update documentation

The Vector.record also presently requires the existence of a Section. This seems unnecessary if the POINT_PROCESS or ARTIFICIAL_CELL arg is present. Section could disagree with POINT_PROCESS about which thread they are in.
It is not easy for the user to determine the thread of an ARTIFICIAL_CELL. Perhaps there should be a method for ARTIFICIAL_CELL and POINT_PROCESS which returns the thread id. Also, recording thread time and associating with a range variable is not obvious to the user. Finally, Vector.record(callable) could be useful.

Copy link

✔️ 0041a06 -> Azure artifacts URL

Copy link

codecov bot commented Apr 25, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 67.19%. Comparing base (88bce8b) to head (e10de94).

Files Patch % Lines
src/nrniv/vrecord.cpp 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2858      +/-   ##
==========================================
+ Coverage   67.18%   67.19%   +0.01%     
==========================================
  Files         563      563              
  Lines      104284   104279       -5     
==========================================
+ Hits        70064    70074      +10     
+ Misses      34220    34205      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bbpbuildbot

This comment has been minimized.

@nrnhines nrnhines marked this pull request as draft April 30, 2024 11:53
@pramodk
Copy link
Member

pramodk commented Apr 30, 2024

The CI was previously green here. I have re-launched CI on this branch just to see if the issue with recent latest failures is code related or runners environment. cc: @alkino

Copy link

sonarcloud bot commented May 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

✔️ e10de94 -> Azure artifacts URL

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.

None yet

3 participants