mrakitin / SRW

Synchrotron Radiation Workshop
Other
0 stars 1 forks source link

Please remove ANSI codes from Makefile #28

Closed robnagler closed 7 years ago

robnagler commented 7 years ago

To learn more, visit respectmyterm.com.

If you want to colorize output, use an output colorizer, e.g. in vim:

:set syntax=sh
mrakitin commented 7 years ago

Does it produce anything bad on Mac? I don't use vim to run the code. The idea was to signal user about unexpected test result while running make test in console.

mrakitin commented 7 years ago

I think I'll use tput there.

robnagler commented 7 years ago

Thanks for using tput. :)

It's about running things in different environments, e.g. if you output to a file. In my case, I run everything from emacs, and it makes it hard to read.

mrakitin commented 7 years ago

OK, I see. I'll give you the version with tput to check later.

robnagler commented 7 years ago

I should note that "make test" does not work, because "timeout" doesn't exist.

I think you should probably write "true" shell scripts and call them from the Makefile as opposed to writing scripts s individual Make commands. It's hard to get that right.

mrakitin commented 7 years ago

Yes, it requires timeout to limit the time of the execution, it's mentioned here - https://github.com/mrakitin/SRW/wiki/Compile-SRW-on-Linux-&-Mac. Any ideas how to improve it?

As for the scripts inside the Makefile, I tried to call an analog from outside, but it was more convenient to have a single file rather than two separate ones - one of the requirements for merge with the main SRW...

robnagler commented 7 years ago

I think the approach has to be feature based. You can't know which version of bash is installed, and the version on Mac OS is ancient and doesn't have "timeout". (Apple won't upgrade to the latest bash for GPL reasons).

To simulation timeout, you have to run the process in background, and kill it with a timer process. It's a bit of work but possible. :)

mrakitin commented 7 years ago

ANSI color codes removed in both https://github.com/mrakitin/SRW/commit/46a38d4174e1717b96564aa3a4e519427d46e163 and https://github.com/radiasoft/SRW-light/commit/52ddf3d06d0c6f5d589922f0fc270692ebf9dd5d.

I'll check timeout upgrade tomorrow.

mrakitin commented 7 years ago

I anyway have this kind of strings if I redirect the output to a file:

if [ $remove_tmp_dir -eq 1 ]; then \
    cd /home/vagrant/src/radiasoft/SRW-light; \
    rm -rf /home/vagrant/src/radiasoft/SRW-light/env/work/srw_python/data_example_10; \
fi;

        Test ^[[32m^[[1mPASSED^[[m^O. Code=124 (timeouted, expected)

Can it be avoided somehow?

robnagler commented 7 years ago

Unfortunately, you need to check if stdout is a tty.

I would recommend you simplify your code, and instead, look at a more powerful IDE, like emacs. :)

mrakitin commented 7 years ago

OK, thanks for the suggestion about tty. I know emacs is great, but for a scientist who want to compile SRW occasionally it may be easier just to do it in a regular console. Anyway, I never saved the output of the test to a file, so I don't think it's a common use case. It's more important to look at the timeout.

mrakitin commented 7 years ago

@robnagler, you could give it a try - mrakitin/SRW (master branch). I addressed the following issues:

Note: if you run Example 10 on Mac for some time (without the test), it fails after the 4th step with the error:

Calculation will be sequential (non-parallel), because "mpi4py" module can not be loaded
i= 0 Electron Coord.: x= -2.74452048352e-05 x'= -5.30201947148e-06 y= 3.33399322641e-06 y'= 8.88316322966e-07 E= 3.00238284361
i= 1 Electron Coord.: x= 2.94014986023e-05 x'= -3.43237532114e-06 y= -1.01652414766e-06 y'= -5.38704099806e-06 E= 3.00112419227
i= 2 Electron Coord.: x= 2.30356693789e-05 x'= 1.81178681222e-06 y= 2.70769424073e-06 y'= 1.69800387734e-06 E= 3.00249151578
i= 3 Electron Coord.: x= -4.12356508492e-05 x'= -2.36273974903e-05 y= 2.75360743031e-06 y'= 4.41985214074e-06 E= 2.99916732752
i= 4 Electron Coord.: x= 6.21094516163e-05 x'= -1.07174990888e-05 y= -1.19992938014e-06 y'= -3.97142680411e-06 E= 3.0004317949
Traceback (most recent call last):
  File "SRWLIB_Example10.py", line 217, in <module>
    os.path.join(os.getcwd(), strExDataFolderName, strIntPropME_OutFileName), sampFactNxNyForProp, optBL)
  File "/Users/vagrant/src/mrakitin/SRW/env/work/srw_python/srwlib.py", line 4593, in srwl_wfr_emit_prop_multi_e
    srwl_uti_save_stat_wfr_emit_prop_multi_e(i + 1, total_num_of_particles, filename=log_path)
  File "/Users/vagrant/src/mrakitin/SRW/env/work/srw_python/srwlib.py", line 3839, in srwl_uti_save_stat_wfr_emit_prop_multi_e
    srwl_uti_save_text(text_to_save, status_text_file, mode=mode)
  File "/Users/vagrant/src/mrakitin/SRW/env/work/srw_python/srwlib.py", line 3697, in srwl_uti_save_text
    with open(_file_path, mode) as f:
IOError: [Errno 2] No such file or directory: '/Users/vagrant/src/mrakitin/SRW/env/work/srw_python/__srwl_logs__/srwl_stat_wfr_emit_prop_multi_e_2017-01-30_19-59-32.log'

I am investigating of the issue - probably the new code for creation of the directory does not work properly on Mac.

mrakitin commented 7 years ago

A brilliant implementation by @robnagler is merged https://github.com/mrakitin/SRW/pull/30. There is still an issue with running SRW on Mac, but it will be solved in a separate ticket.

robnagler commented 7 years ago

I'm not sure how brilliant, but it does handle the timeout issue. :)