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

test_netcvode.py: increase comparison tolerances. #1822

Merged
merged 1 commit into from
May 17, 2022

Conversation

olupton
Copy link
Collaborator

@olupton olupton commented May 16, 2022

- This makes the test pass when NEURON is compiled with the NVIDIA
  compilers, nvc and nvc++.
- Using math.isclose for the comparison reduces the number of h.Vector
  objects that are constructed; modify the reference file to take that
  into account.
@olupton olupton requested a review from nrnhines May 16, 2022 10:33
Copy link
Member

@nrnhines nrnhines left a comment

Choose a reason for hiding this comment

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

The question of whether it's best to use absolute or relative tolerance is hard to resolve. I think generally we are looking for accuracy over the dynamic range of each state. I.e. a channel gating state range is 0-1 and errors near 0, or .5, or 1 are in my opinion equivalent. Voltage generally ranges from -100 to 100mV and there is nothing special about 0. Concentrations range from >0 to 100mM but calcium is often special and we want it to be accurate in the 10-6 mM (nanomolar) range. I've written pump models in which the dynamic range was on the order of >0 to 1e-15 . So with CVode I'm often selecting atolscale for each state type based on the maximum absolute value during a sim for each state type (rounded to nearest order of magnitude). The the atol multiplier gives an overall indication of the accuracy desired in the sense that if atol is reduced by a factor of 10, the overall accuracy of the entire sim increases by a factor of 10. For testing, it's delightful if the results are identical . Otherwise, I guess relative tolerance is the best way to compare.

Seems like the nvidia change to number of time steps needs to be investigated but I'm happy to have that put into a separate issue.

@nrnhines nrnhines merged commit c127b54 into master May 17, 2022
@nrnhines nrnhines deleted the olupton/nvhpc-tolerances branch May 17, 2022 10:50
@alexsavulescu alexsavulescu mentioned this pull request Jun 29, 2022
19 tasks
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.

2 participants