pace-neutrons / Horace

Horace is a suite of programs for the visualization and analysis of large datasets from time-of-flight neutron inelastic scattering spectrometers.
https://pace-neutrons.github.io/Horace/stable/
GNU General Public License v3.0
8 stars 5 forks source link

Identify all "dead" code #129

Closed nickbattam-tessella closed 4 years ago

nickbattam-tessella commented 4 years ago

The Horace/Herbert code base includes a number of deprecated, redundant and superseded methods and classes.

Some of the currently failing unit tests are testing that code (e.g. those dealing with mslice)

Outcome:

Dependent on Herbert issue []#101 Dependent on #158

nickbattam-tessella commented 4 years ago

From Toby:

I think we can ditch all the code related to mslice, however. We want to make mslice redundant in that all functionality of the old mslice will end up in PACE – generalised projections will give proper powder functionality (which is currently only fudged in Horace). On reflection, I agree that keeping the mslice code around is just a millstone. I think the only thing we want to be able to do is easily find the old mslice code in the repository should we ever want to pull some of it back.

\Herbert\herbert_core\classes\mslice_classes\ I think all of this can go. These are the class definitions for the mslice objects (cut, slice, spe), and the associated detector files (.par and .phx). Horace uses data_loaders factory to load all kinds of .par, phx and detectors from nxspe file.

\Herbert\_test\test_mslice_objects\ \Horace\_test\test_mslice_utilities\ I think all these tests can go as well.

nickbattam-tessella commented 4 years ago

From Toby (on FORTRAN code):

\Herbert\_LowLevelCode\Fortran

\projects These are the Visual studio projects - I think Alex has been looking after these, even if I created them originally (which I don't recall now if I did or not)

\source\file_io I think all of this can go – it only concerns mslice cuts and slices

\source\maths \source\tools KEEP: This is all related to Herbert functions for IX_dataset_1d, _2, 3d that perform rebinning and integration operations.

\source\type_definitions.f90 KEEP: Some definitions used in the fortran to be retained

\source_mex Mex files that use the code in \source. Most source files are still needed, but we can delete get_cut_fortran.F, get_slice_fortran, get_spefortran.F, and the three put* equivalents

\source_mex_interface KEEP: Needed by other Herbert functions

nickbattam-tessella commented 4 years ago

From Toby: We can delete all of \Herbert\herbert_core\compatibility which has some shortcut functions for what are now obsolete applications mgenie and libisis

nickbattam-tessella commented 4 years ago

From Toby: We should keep both uv_correct and uv_correct2. Although originally inspired by the needs of mslice, they actually perform a function that is not specific to mslice. The naming of the functions is unfortunate, I concede, but their function is useful. They allow the parameters of a reoriented crystal to be converted into the true u and v vectors, something I have done before when regenerating sqw files.

nickbattam-tessella commented 4 years ago

From Toby:

I’ve been running the timing tests for Herbert rebin operations, and seeing under what conditions the Matlab gets excessively slow. My conclusions are

Removing the fortran means a number of routines with blocks of code

if use_mex
    try
        :
    catch ERR
        if ~force_mex
            fprintf('Error %s calling mex function %s_mex. Calling matlab equivalent\n',ERR.message,mfilename);
            use_mex=false;
        else
            x_out=[];
            ok=false;
        end
    end
end

if ~use_mex
    :
    if ~ok
        ERR = MException('IX_dataset:invalid_argument',mess);
    end
end

will need to be edited to just call the matlab routine.

*: the time-consuming Matlab is for the case of rebinning point data only, when the trapezoidal integration method is used.

ghost commented 4 years ago

All sub-issues closed