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

Investigate models failing nrntest run after CPP update #951

Closed
alexsavulescu opened this issue Feb 1, 2021 · 11 comments · Fixed by #763
Closed

Investigate models failing nrntest run after CPP update #951

alexsavulescu opened this issue Feb 1, 2021 · 11 comments · Fixed by #763
Assignees

Comments

@alexsavulescu
Copy link
Member

From #763:

b04feb12 35358
splitcell 97917
b08dec23 116830
b07dec27_20091025 106891
fdemo 138379 # don't worry about this one
b07dec17 105507
gap-modeldb 43039
nrntraub 82894
lytton98 7399
lytton97 9889
wang1996 26997
prknj 17664
b09jan08 116838
lytton99 7400

For reference:

Download those models and fix the nocmodl issues. If you do one, the others will likely fall into line.

The prknj differences are so small (1e-8) that they can be ignored.
The gap-modeldb differences aren't so small but the lines on the graphs overlap so I think it is acceptable. And i will diagnose the differences when I get time.
I think the fdemo differences are due to unique random seed on every run.
splitcell has empty gout because of rand.c compile errors, e.g.

rand.c:452:4: note: previous definition of ‘z’ was here
   (z)[0] = LOW(l); (z)[1] = HIGH(l); }
    ^
@alexsavulescu alexsavulescu self-assigned this Feb 1, 2021
@alexsavulescu
Copy link
Member Author

I've checked all models.

  • new issues don't seem familiar to me at all (at least I don't recall seeing them while running nrnivmodl@cpp on ModelDBRepo, but after that came some CR commits and some merges). I get the feeling there must be a common root cause though.
  • some of them fail even for nrnivmodl@master

I will continue investigating tomorrow.

@pramodk
Copy link
Member

pramodk commented Feb 1, 2021

@alexsavulescu : ok, lets take a look together tomorrow.

@alexsavulescu
Copy link
Member Author

Ok so I believe I got to the root of this. One change following the CR was interfering with the generated macros by adding one extra empty line \n. That lead to some crazy compile errors.. Will re-run models listed in this ticket to make sure there is no actual regression or anything else.

@alexsavulescu
Copy link
Member Author

With fix dc62f2f nrn@master vs nrn@cpp behave consistently.

@alexsavulescu alexsavulescu linked a pull request Feb 2, 2021 that will close this issue
@alexsavulescu
Copy link
Member Author

@nrnhines would it be easy to run again the tests for the aforementioned models? Otherwise I believe #763 is good for merge.

@nrnhines
Copy link
Member

nrnhines commented Feb 2, 2021

Will do so. Will know result in a couple hours.

@alexsavulescu
Copy link
Member Author

Will do so. Will know result in a couple hours.

Great, thanks.

@nrnhines
Copy link
Member

nrnhines commented Feb 2, 2021

Still getting exactly the same problems with 8.0a-32-g4186296c . Do I have the right version?

@nrnhines
Copy link
Member

nrnhines commented Feb 2, 2021

Nevermind. I botched the test. Will try again.

@nrnhines
Copy link
Member

nrnhines commented Feb 2, 2021

All the "no such file or directory" issues are gone.

@pramodk
Copy link
Member

pramodk commented Feb 2, 2021

🎉

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 a pull request may close this issue.

3 participants