search

neuronsimulator / nrn-modeldb-ci

NEURON ModelDB CI tools
BSD 3-Clause "New" or "Revised" License
4 stars 3 forks source link

Clean up differences between neuron and neuron-nightly #77

Closed olupton closed 1 year ago

olupton commented 1 year ago

Many of the diffs are due to error messages that are, in essence, the same on both sides of the comparison. Some error message formatting has changed since NEURON 8.2.2, so "the same error" can still result in a diff. This is an effort to avoid the problem by fixing the errors, with obvious benefits.

To be merged first + github: keys removed from YAML in this PR:

olupton commented 1 year ago

The latest run only has one model showing up in the diff

Screenshot 2023-02-28 at 11 53 34

when comparing 8.2.2 with neuron-nightly. Note that this does not include the gout comparison, but I believe that this also shows no diffs when I run it locally.

On the face of it this is a more substantive issue that needs further investigation. I suggest we can move ahead and merge this PR as soon as the dependent updates to models linked in the description are merged.

olupton commented 1 year ago

The latest run only has one model showing up in the diff Screenshot 2023-02-28 at 11 53 34 when comparing 8.2.2 with neuron-nightly. Note that this does not include the gout comparison, but I believe that this also shows no diffs when I run it locally.

On the face of it this is a more substantive issue that needs further investigation. I suggest we can move ahead and merge this PR as soon as the dependent updates to models linked in the description are merged.

This diff is removed by https://github.com/ModelDBRepository/150245/pull/3/commits/9e033d44df8a77458f4afed1fb84b290d9130ca7. In my local setup, the uninitialised value in 8.2.2 was 25, while in neuron-nightly it was zero.

RuiLi7222 commented 1 year ago

Many of the diffs are due to error messages that are, in essence, the same on both sides of the comparison. Some error message formatting has changed since NEURON 8.2.2, so "the same error" can still result in a diff. This is an effort to avoid the problem by fixing the errors, with obvious benefits.

To be merged first + github: keys removed from YAML in this PR:

All the approved PRs above have been merged and updated to ModelDB.

However, the drat pull request of the three models - 146949, 150245, and 267067 have very similar changes as the approved PR. Do you plan to continue working on the draft PR, or do you like to delete them? @olupton

olupton commented 1 year ago

Many of the diffs are due to error messages that are, in essence, the same on both sides of the comparison. Some error message formatting has changed since NEURON 8.2.2, so "the same error" can still result in a diff. This is an effort to avoid the problem by fixing the errors, with obvious benefits. To be merged first + github: keys removed from YAML in this PR:

All the approved PRs above have been merged and updated to ModelDB.

However, the drat pull request of the three models - 146949, 150245, and 267067 have very similar changes as the approved PR. Do you plan to continue working on the draft PR, or do you like to delete them? @olupton

Thanks for merging the updates! Those other draft PRs are connected to neuronsimulator/nrn#2027; please leave them open for the moment, they will be updated soon.

olupton commented 1 year ago

Launched https://github.com/neuronsimulator/nrn-modeldb-ci/actions/runs/4325619099 with the full set of models.