Myers diff algorithm for unit tests #1161
Conversation
For now just one test case was updated as an example for initial feedback
This comment has been minimized.
This comment has been minimized.
|
I'd propose thorough (unit-) testing. |
test/unit/visitor/loop_unroll.cpp
Outdated
| REQUIRE(reindent_text(output_nmodl) == reindent_text(result)); | ||
| if (reindent_text(output_nmodl) != reindent_text(result)) { | ||
| auto diff = MyersDiff(reindent_text(output_nmodl), reindent_text(result)); | ||
| diff.do_diff(); |
There was a problem hiding this comment.
This pattern rather reliably tells you that MyersDiff is either not a class or not initializing itself correctly. Probably, the easiest would be to just call do_diff from inside its ctor.
Then provide means for creating a string; either MyersDiff::to_string() or overload operator<<.
Finally, probably provide a function that combines the two so this can be at most just:
std::string diff = myers_diff(output_nmodl, result);
if(diff) {
FAIL(diff);
}
There was a problem hiding this comment.
OK, thanks. Yes, this all makes sense.
On the last point, though.. unfortunately the algorithm (or at least my implementation) currently still sometimes shows some stupid diffs, e.g. a line that is the same in a and b can sometimes show up as deleted and added.
This might be really just a bug that I need to address, but it could be also that some of those corner cases will remain, which would show as false-positives in the unit test.
There was a problem hiding this comment.
The entire thing is complex, mostly C and doesn't have a single test. Personally, I would be unable to implement this algorithm even broadly correctly without quite thorough testing.
It's all rather a lot of work for a subprocess.run(["diff", a, b]) (in tests we can assume we have access python).
and code formatting
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…tatement (#1125) * Support for new RANDOM construct in NMODL, see neuronsimulator/nrn#2627 * Added necessary changes to the parser, visitors, symbol table and code generation * Added unit tests Co-authored-by: Pramod S Kumbhar <pramod.s.kumbhar@gmail.com>
This commits move various ion-related code from the CoreNEURON C++ printer to the common base of both C++ printers. There are some small changes whenever stubbing is needed.
This implements a trivial case of reading an ion variable `ena` and writing `42.0` to `ina`; along with the functionality required to be able to record `ina`.
Created FunctionCallpathVisitor that traverses the function calls and sets `use_range_ptr_var` property
* Added hoc_register_prop_size and hoc_register_dparam_semantics * Implemented big part of the nrn_alloc to make POINT_PROCESS work * Added test for POINT_PROCESS location * Added test for parameters
* Added support for the usage of FUNCTION and PROCEDURES * hoc and python wrappers registration also done * Corresponding tests added
Use `imgmath_embed = True` to work around a path bug for files/notebooks located in subfolders. Use `imgmath_image_format = 'svg'` because PNG results in pixalated images.
The removed test only checks if certain substrings are present. The golden testing automatically adds these tests for any usecase. Therefore, this test is not needed anymore. The usecase for implementing non-specific current, will add a version of a passive current; and test the values against the analytical reference. Ions are already covered by the `ionic` usecase. Array variables are covered by the `cnexp_array` usecase. Parameters coved by the `parameters` usecase. Simple ODEs are covered by `cnexp_*` usecases.
For functions that are meant to be called "in a loop", i.e. function which accept `id`, we also pass `inst`.
Starts write up of the underlying equations as needed in NMODL.
This commits adds a test for NONSPECIFIC_CURRENT and implements the codegen for NEURON required to pass the test. It does not set the conductances.
* Update to latest hpc-coding-conventions * Pin version 13 for clang-format to avoid code reformatting
* replace `setup.py` with `pyproject.toml` * move all Python package requirements to single `requirements.txt` * remove checking Python package requirements in `CMakeLists.txt` * move Python bindings to own dir (python/nmodl), fixes #462 * add separate script for generating docs * add `packaging/change_name.py` script to as workaround to enable both NMODL and NMODL-nightly wheels in CI * update documentation and CI to reflect above changes --------- Co-authored-by: Luc Grosheintz <luc.grosheintz@gmail.com>
* use cibuildwheel for creating redistributable wheels * simplify CI pipeline * update packaging docs --------- Co-authored-by: Nicolas Cornu <nicolas.cornu@epfl.ch> Co-authored-by: Luc Grosheintz <luc.grosheintz@gmail.com>
* Throw error if passing directory instead of file to `parse_file` * Add test for it
The version in the runner and expected by homebrew regularly get out-of-sync and then crash the CI. The workaround is to attempt updating python; then force the rest of the python update; and then continue with the actual `brew install`.
The previous behaviour for
KINETIC states {
~ x + y << 42.0
}
was to print a warning and continue as if the line didn't exist. The new
behaviour is to throw an error, because it's unclear why ignoring the
line could ever lead to a correct translation of the MOD file.
* Format Python files with `black`. * Enable `black` formatter.
What's meant is things like `celsius`.
When encountering an indexed `COMPARTMENT` block:
COMPARTMENT i, volume_expr { species }
All state variables are searched if they match they match the pattern
`"{species}[%d]"`. For each matching state variables, we obtain the
value of the array index and substitute the name of the array index with
its value in `volume_expr`. This is then stored in the array of
compartment factors.
Check that the resulting derivatives are divided by the volume
of the compartment.
Recent changes to Github mean that these runs are cancelled for no apparent reason. We're turning off these runs, to be able to investigate/fix the issue without blocking other work.
catch2: v3.3.2 to v3.5.3 cli11: v2.2.0 to v2.4.1 json: v3.10.5 to v3.11.3 spdlog: v1.12.0 to v1.13.0
|
Logfiles from GitLab pipeline #212074 (:no_entry:) have been uploaded here! Status and direct links: |
|
I'll close this. As discussed, it's not very high priority and unless someone has interest and finds the time this can just stay in a branch. |
This is a small new feature for a bit of convenience. This PR adds a proper diff implementation that can be invoked when a unit test fails. Instead of outputting the the raw generated strings and painstakingly comparing them, we can now simply diff them and print formatted output.
For now just one test case was updated as an example for initial feedback.