crash when running mh_metric with html output

Remi-Gau commented 3 years ago

MISS_HIT Component affected

Code metrics

Your operating system and Python version

Linux: ubuntu 18.04
State output of python3 --version: Python 3.8.0

With MISS_HIT 0.9.20

Note: The python version is from in my venv though the traceback (see below) mentions that I am miss_hit is running from python 3.6: not sure how to change that (still not quite clear with managing virtual environments)...

Describe the bug

Simply tried to run metrics with HTML output after cloning a repo. No special config.

An html file is created but I get a crash (see below)

"Core" to run

git clone https://github.com/spm/spm12 tmp/spm12
mh_metric --html tmp.html tmp/spm12

Traceback

Traceback (most recent call last):
  File "/home/remi/.local/bin/mh_metric", line 8, in <module>
    sys.exit(main())
  File "/home/remi/.local/lib/python3.6/site-packages/miss_hit_core/mh_metric.py", line 1188, in main
    command_line.ice_handler(main_handler)
  File "/home/remi/.local/lib/python3.6/site-packages/miss_hit_core/command_line.py", line 363, in ice_handler
    main_function()
  File "/home/remi/.local/lib/python3.6/site-packages/miss_hit_core/mh_metric.py", line 1184, in main_handler
    command_line.execute(mh, options, {}, metric_backend)
  File "/home/remi/.local/lib/python3.6/site-packages/miss_hit_core/command_line.py", line 357, in execute
    back_end.post_process()
  File "/home/remi/.local/lib/python3.6/site-packages/miss_hit_core/mh_metric.py", line 1083, in post_process
    worst_offenders)
  File "/home/remi/.local/lib/python3.6/site-packages/miss_hit_core/mh_metric.py", line 736, in write_html_report
    results = metrics["metrics"][file_metric]
KeyError: 'file_length'

florianschanda commented 3 years ago

So there are parse errors that I am getting when trying to reproduce this, can you confirm please that you get them too?

In spm_eeg_review_callbacks.m, line 749
|                 try rotate3d off;end
|                              ^^^ error: expected end of statement, found IDENTIFIER instead
In spm_fx_hdm.m, line 76
| try, global dt, f  = f*dt; end
|               ^ error: expected IDENTIFIER, found COMMA instead
@file_array/Contents.m: error: expected KEYWORD, reached EOF instead
@gifti/Contents.m: error: expected KEYWORD, reached EOF instead
@meeg/Contents.m: error: expected KEYWORD, reached EOF instead
@nifti/Contents.m: error: expected KEYWORD, reached EOF instead
@xmltree/Contents.m: error: expected KEYWORD, reached EOF instead
In external/fieldtrip/fileio/ft_read_data.m, line 1540
|     case'chans_samples_trials'
|         ^ lex error: unable to distinguish between string and transpose operation
In external/fieldtrip/preproc/ft_preproc_online_downsample_init.m, line 27
| if factor < 1 or factor~=round(factor)
|                  ^^^^^^ error: expected end of statement, found IDENTIFIER instead
In toolbox/DEM/spm_cost_SHC_fx.m, line 22
| global A; X   = A.x;
|           ^ error: expected NEWLINE, found IDENTIFIER instead
In toolbox/DEM/spm_cost_SHC_fxa.m, line 24
| global A; X   = A.x(:,A.q);
|           ^ error: expected NEWLINE, found IDENTIFIER instead
In toolbox/DEM/spm_fx_hdm_sck.m, line 55
| try, global dt, f  = f*dt; end
|               ^ error: expected IDENTIFIER, found COMMA instead
In toolbox/DEM/spm_voice_fundamental.m, line 30
| global VOX, try VOX.formant; catch, VOX.formant = 0; end
|           ^ error: expected IDENTIFIER, found COMMA instead
In toolbox/FieldMap/pm_invert_phasemap.m, line 111
|          for i=1:indx(1)-1 y(i) = y(indx(1)); end
|                            ^ error: expected end of statement, found IDENTIFIER instead

florianschanda commented 3 years ago

So there are two issues I think:

the parse errors are the cause of the original error you're seeing. MH Metric should not attempt to build metrics for something it can't parse. I've had this issue elsewhere and I thought I fixed it, but apparently not everywhere :(
The above seems to be valid but horrific (is there ever a case where this is not the case?) MATLAB, so I should not raise parse errors

I will fix the first issue under this ticket, and I will create new tickets for the parse errors.

florianschanda commented 3 years ago

I will fix the various parse issues under https://github.com/florianschanda/miss_hit/issues/219

Remi-Gau commented 3 years ago

So there are parse errors that I am getting when trying to reproduce this, can you confirm please that you get them too?

yup that is correct.

disclaimer though: that codebase is not "mine".

I am trying to try to create a miss_hit "gallery" by running miss_hit on some common toolboxes in my field and creating vizualization of the output: hoping to show how static analysis can help with keeping one's codebase "clean" (and also that one's code does not need to be perfect to be shared)

I guess it is also turning into a "stress test" for miss_hit.

florianschanda commented 3 years ago

There are some... "interesting" constructs in that codebase. I hope to be able to sort out most, but there is one or two that I may not be able to ever do since it would require a radically different parsing/lexing architecture.

You see I think the MATLAB language predates the modern "dragon-style" compiler architectures, where you have a totally separate lexing and parsing phase. Basically all languages today are designed in a way to enable this, as it's much easier and safer to write a working compiler this way.

But in MALTAB you basically need to lex and parse at the same time. In MISS_HIT I managed to achieve dragon-style separation by extreme accident; but the lexer is likely the most horrific piece of code I ever wrote. And it is likely that I've made assumptions that may not be true, strictly speaking, in the real world. It was also somewhat hard, since there is no actual official MATLAB documentation/grammar; so I had to reverse engineer and guess at everything. :/

In all these cases I always err on the side of caution, so you get parse errors instead of silently mis-parsing the program (which I think is probably what most people want).

florianschanda commented 3 years ago

I am trying to try to create a miss_hit "gallery" by running miss_hit on some common toolboxes in my field and creating vizualization of the output: hoping to show how static analysis can help with keeping one's codebase "clean" (and also that one's code does not need to be perfect to be shared)

I guess it is also turning into a "stress test" for miss_hit.

This is a great idea. When I released MH for the first time I just picked the three highest starred MATLAB repos from GitHub and made sure it works for these, but because I am actually not a real MATLAB user myself, so I never knew what kind of weird stuff you can do in it.

I really appreciate your efforts, and please keep feeding me more bug reports like this one if you can find them :)

florianschanda / miss_hit

crash when running mh_metric with html output #218