Skip to content
This repository was archived by the owner on Feb 26, 2025. It is now read-only.

Conversation

@DrTaDa
Copy link
Contributor

@DrTaDa DrTaDa commented Jun 23, 2022

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jun 23, 2022

Codecov Report

Merging #406 (748353f) into master (dd5713d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #406   +/-   ##
=======================================
  Coverage   90.49%   90.49%           
=======================================
  Files          41       41           
  Lines        2473     2473           
=======================================
  Hits         2238     2238           
  Misses        235      235           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd5713d...748353f. Read the comment docs.

@DrTaDa DrTaDa requested a review from wvangeit June 23, 2022 13:10
@DrTaDa
Copy link
Contributor Author

DrTaDa commented Jun 23, 2022

@wvangeit to fix the issue with #393

tox.ini Outdated
flake8
mock
neuron-nightly
neuron
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer keep on testing on nightly though, to find issues as early as possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we just found an issue then :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

that was the point ;-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it something we have to report to the nrn github ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably best. This is an error generator by a neuron header file, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The issue is coming from this line:

void* nrn_random_arg(int argpos);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah Pramod answered: neuronsimulator/nrn#1875

Copy link

@pramodk pramodk Jun 23, 2022

Choose a reason for hiding this comment

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

As mentioned in the BBP internal slack channel, assuming this StochKv3.mod from BBP, I would copy the updated version from BBP models (e.g. here)

Would be happy to help if anything else is required.

Edit: And would recommend keep using neuron-nightly so that we can find issues earlier and fix them in advance (on neuron side as well!) :)

@DrTaDa DrTaDa force-pushed the fix_test_CMA branch 3 times, most recently from 99ac27f to 9a473b0 Compare June 24, 2022 08:32
@wvangeit
Copy link
Contributor

I see that there are quite a few changes in examples/l5pc/mechanisms/ProbAMPANMDA_EMS.mod and GABA. I think we don't use these files? Maybe it's better to just remove these from the repo

@wvangeit
Copy link
Contributor

Same for stochkv and stochkv3, we probably only use 1 of these? Best to remove the one we don't use.

@DrTaDa
Copy link
Contributor Author

DrTaDa commented Jun 24, 2022

We use both in the examples/stochkv

@wvangeit
Copy link
Contributor

Ok, but not the synapse files i assume, right?

@DrTaDa
Copy link
Contributor Author

DrTaDa commented Jun 24, 2022

Yes, I removed these

assert abs(log.select("avg")[-1] - 40.) < 1e-4
assert abs(log.select("std")[-1] - 16.32993) < 1e-4
assert pop[0] == [0.09601241274168831, 0.024646650865379722]
assert abs(pop[0][0] - 0.09601241274168831) < 1e-6
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks !

@wvangeit wvangeit merged commit 1e78a88 into master Jun 24, 2022
@wvangeit wvangeit deleted the fix_test_CMA branch June 24, 2022 10:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants