[BUG] Arguments not parsed properly when using the GUI

[DSimonne]

Describe the bug Some arguments, such as

To Reproduce Steps to reproduce the behavior:

1. Use main branch of bcdi when using the GUI
2. Run pre or postprocessing

Expected behavior A clear and concise description of what you expected to happen.

Rotating SIXS data ...
Calling merlin the enchanter in SBS...
Data well rotated by 90°.
Saving example figures...

#########################################################################################

Running: /data/id01/inhouse/david/py38-dev/bin/bcdi_preprocess_BCDI.py
Config file: /gpfs/easy/data/id01/inhouse/david/UM2022/SixS/TestGui/S3116/preprocessing/config_preprocessing.yml

#########################################################################################

---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/gwaihir/gui/gui.py in action_button_run_preprocess(selfbutton)
   3286                 config_file = self.preprocessing_folder + "/config_preprocessing.yml"
   3287                 parser = ConfigParser(config_file, cli_args)
-> 3288                 args = parser.load_arguments()
   3289                 args["time"] = f"{datetime.now()}"
   3290                 run_preprocessing(prm=args)

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/parser.py in load_arguments(self)
    246 
    247         # validate the parameters
--> 248         self.arguments = self._check_args(args)
    249         return self.arguments
    250 

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/parser.py in _check_args(dic)
    214         checked_keys = []
    215         for key, value in dic.items():
--> 216             value, is_valid = valid_param(key, value)
    217             if is_valid:
    218                 dic[key] = value

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/parameters.py in valid_param(key, value)
     89         "use_rawdata",
     90     }:
---> 91         valid.valid_item(value, allowed_types=bool, name=key)
     92     elif key == "absorption":
     93         valid.valid_item(value, allowed_types=Real, min_excluded=0, name=key)

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/validation.py in valid_item(value, allowed_types, min_included, min_excluded, max_included, max_excluded, allow_none, name)
    270     # check the type of obj
    271     if value is not None and not isinstance(value, allowed_types):
--> 272         raise TypeError(
    273             f"{name}: wrong type for value, "
    274             f"allowed is {allowed_types}, got {type(value)}"

TypeError: flip_reconstruction: wrong type for value, allowed is (<class 'bool'>,), got <class 'str'>

Used on slurm, or locally.

[DSimonne]

It's OK when using the devel branch on my fork of bcdi

[DSimonne]

In postprocessing:

#########################################################################################

Running: /data/id01/inhouse/david/py38-dev/bin/bcdi_strain.py
Config file: /gpfs/easy/data/id01/inhouse/david/UM2022/SixS/TestGui/S3116/postprocessing//config_postprocessing.yml

#########################################################################################

'pixel_size' is an unexpected key, its value won't be considered.
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/ipywidgets/widgets/interaction.py in update(self, *args)
    254                     value = widget.get_interact_value()
    255                     self.kwargs[widget._kwarg] = value
--> 256                 self.result = self.f(**self.kwargs)
    257                 show_inline_matplotlib_plots()
    258                 if self.auto_display and self.result is not None:

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/gwaihir/gui/gui.py in initialize_postprocessing(self, unused_label_averaging, sort_method, correlation_threshold, unused_label_FFT, phasing_binning, original_size, preprocessing_binning, output_size, keep_size, fix_voxel, unused_label_disp_strain, data_frame, save_frame, ref_axis_q, isosurface_strain, strain_method, phase_offset, phase_offset_origin, offset_method, centering_method, unused_label_refraction, correct_refraction, optical_path_method, dispersion, absorption, threshold_unwrap_refraction, unused_label_options, simulation, invert_phase, flip_reconstruction, phase_ramp_removal, threshold_gradient, save_raw, save_support, save, debug, roll_modes, unused_label_data_vis, align_axis, ref_axis, axis_to_align, strain_range, phase_range, grey_background, tick_spacing, tick_direction, tick_length, tick_width, unused_label_average, averaging_space, threshold_avg, unused_label_apodize, apodize, apodization_window, half_width_avg_phase, apodization_mu, apodization_sigma, apodization_alpha, unused_label_strain, strain_folder, reconstruction_file, run_strain)
   4998                 config_file = self.postprocessing_folder + "/config_postprocessing.yml"
   4999                 parser = ConfigParser(config_file, cli_args)
-> 5000                 args = parser.load_arguments()
   5001                 args["time"] = f"{datetime.now()}"
   5002                 run_postprocessing(prm=args)

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/parser.py in load_arguments(self)
    246 
    247         # validate the parameters
--> 248         self.arguments = self._check_args(args)
    249         return self.arguments
    250 

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/parser.py in _check_args(dic)
    214         checked_keys = []
    215         for key, value in dic.items():
--> 216             value, is_valid = valid_param(key, value)
    217             if is_valid:
    218                 dic[key] = value

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/parameters.py in valid_param(key, value)
     89         "use_rawdata",
     90     }:
---> 91         valid.valid_item(value, allowed_types=bool, name=key)
     92     elif key == "absorption":
     93         valid.valid_item(value, allowed_types=Real, min_excluded=0, name=key)

/data/id01/inhouse/david/py38-dev/lib/python3.8/site-packages/bcdi/utils/validation.py in valid_item(value, allowed_types, min_included, min_excluded, max_included, max_excluded, allow_none, name)
    270     # check the type of obj
    271     if value is not None and not isinstance(value, allowed_types):
--> 272         raise TypeError(
    273             f"{name}: wrong type for value, "
    274             f"allowed is {allowed_types}, got {type(value)}"

TypeError: flip_reconstruction: wrong type for value, allowed is (<class 'bool'>,), got <class 'str'>

[carnisj]

It's OK when using the devel branch on my fork of bcdi

So you say that the devel branch of your GUI works fine but not your main? Is it really related to bcdi?

[DSimonne]

Hi, sorry I mean the devel branch on my fork of bcdi. Mostly because of this edit:

        "use_rawdata",
    }:
        valid.valid_item(value, allowed_types=bool, name=key)
    elif key == "flip_reconstruction":
        pass
        # print(key, value)
        # valid.valid_item(value, allowed_types=bool, name=key)
    elif key == "absorption":
        valid.valid_item(value, allowed_types=Real, min_excluded=0, name=key)
    elif key == "actuators":

[carnisj]

Yes we already saw that you are getting a weird string instead of the expected boolean, but why is the question ...

[DSimonne]

Indeed ^^ And if Vincent kept your bcdi when installing it won't work

[carnisj]

can you try with the branch bug_parsing. I simplified the conversion of string "True" to boolean. If this still doesn't work I could only think of a weird hidden ASCII character (space or so) in your config file

[DSimonne]

can you try with the branch bug_parsing. I simplified the conversion of string "True" to boolean. If this still doesn't work I could only think of a weird hidden ASCII character (space or so) in your config file

Does not work either, same error ;'(

[DSimonne]

I checked the files for any hiddend character but can't find one :(

[carnisj]

can you try with the branch bug_parsing. I simplified the conversion of string "True" to boolean. If this still doesn't work I could only think of a weird hidden ASCII character (space or so) in your config file

Does not work either, same error ;'(

Then I would first write a new config file from scratch (no copy-paste) and check.

[carnisj]

How are you setting the parameter "flip_reconstruction", from the command line or from the config file? I added some unit tests but until now the program works as expected. What we need to understand is where this weird string comes from.

[DSimonne]

Method that creates the file

    @ staticmethod
    def create_yaml_file(fname, **kwargs):
        """Create yaml file storing all keywords arguments given in input Used
        for bcdi scripts.

        :param fname: path to created yaml file
        """
        config_file = []

        for k, v in kwargs.items():
            if isinstance(v, str):
                config_file.append(f"{k}: \"{v}\"")
            elif isinstance(v, tuple):
                if v:
                    config_file.append(f"{k}: {list(v)}")
                else:
                    config_file.append(f"{k}: None")
            elif isinstance(v, np.ndarray):
                config_file.append(f"{k}: {list(v)}")
            elif isinstance(v, list):
                if v:
                    config_file.append(f"{k}: {v}")
                else:
                    config_file.append(f"{k}: None")
            else:
                config_file.append(f"{k}: {v}")

        file = os.path.basename(fname)
        directory = fname.strip(file)
        # Create directory
        if not os.path.isdir(directory):
            full_path = ""
            for d in directory.split("/"):
                full_path += d + "/"
                try:
                    os.mkdir(full_path)
                except FileExistsError:
                    pass

        with open(fname, "w") as v:
            for line in config_file:
                v.write(line + "\n")

Calling the method

# Create config file
self.create_yaml_file(
    fname=f"{self.preprocessing_folder}config_preprocessing.yml",
    scans=self.Dataset.scan,
    root_folder=root_folder,
    save_dir=self.preprocessing_folder,
    data_dir=data_dir,
    sample_name=self.Dataset.sample_name,
    comment=self.Dataset.comment,
    debug=self.Dataset.debug,
    # parameters used in masking
    flag_interact=self.Dataset.flag_interact,
    background_plot=self.Dataset.background_plot,
    backend=self.matplotlib_backend,
    # parameters related to data cropping/padding/centering
    centering_method=self.Dataset.centering_method,
    fix_size=self.Dataset.fix_size,
    center_fft=self.Dataset.center_fft,
    pad_size=self.Dataset.pad_size,
    # parameters for data filtering
    mask_zero_event=self.Dataset.mask_zero_event,
    median_filter=self.Dataset.median_filter,
    median_filter_order=self.Dataset.median_filter_order,
    # parameters used when reloading processed data
    reload_previous=self.Dataset.reload_previous,
    reload_orthogonal=self.Dataset.reload_orthogonal,
    preprocessing_binning=self.Dataset.preprocessing_binning,
    # saving options
    save_rawdata=self.Dataset.save_rawdata,
    save_to_npz=self.Dataset.save_to_npz,
    save_to_mat=self.Dataset.save_to_mat,
    save_to_vti=self.Dataset.save_to_vti,
    save_as_int=self.Dataset.save_as_int,
    # define beamline related parameters
    beamline=self.Dataset.beamline,
    actuators=self.Dataset.actuators,
    is_series=self.Dataset.is_series,
    rocking_angle=self.Dataset.rocking_angle,
    specfile_name=self.Dataset.specfile_name,
    # parameters for custom scans
    custom_scan=self.Dataset.custom_scan,
    custom_images=self.Dataset.custom_images,
    custom_monitor=self.Dataset.custom_monitor,
    # detector related parameters
    detector=self.Dataset.detector,
    phasing_binning=self.Dataset.phasing_binning,
    linearity_func=self.Dataset.linearity_func,
    # center_roi_x
    # center_roi_y
    roi_detector=self.Dataset.roi_detector,
    normalize_flux=self.Dataset.normalize_flux,
    photon_threshold=self.Dataset.photon_threshold,
    photon_filter=self.Dataset.photon_filter,
    bin_during_loading=self.Dataset.bin_during_loading,
    frames_pattern=self.Dataset.frames_pattern,
    background_file=self.Dataset.background_file,
    hotpixels_file=self.Dataset.hotpixels_file,
    flatfield_file=self.Dataset.flatfield_file,
    template_imagefile=self.Dataset.template_imagefile,
    # define parameters below if you want to orthogonalize the
    # data before phasing
    use_rawdata=self.Dataset.use_rawdata,
    interpolation_method=self.Dataset.interpolation_method,
    fill_value_mask=self.Dataset.fill_value_mask,
    beam_direction=self.Dataset.beam_direction,
    sample_offsets=self.Dataset.sample_offsets,
    sdd=self.Dataset.sdd,
    energy=self.Dataset.energy,
    custom_motors=self.Dataset.custom_motors,
    # parameters when orthogonalizing the data before
    # phasing  using the linearized transformation matrix
    align_q=self.Dataset.align_q,
    ref_axis_q=self.Dataset.ref_axis_q,
    direct_beam=self.Dataset.direct_beam,
    dirbeam_detector_angles=self.Dataset.dirbeam_detector_angles,
    bragg_peak=self.Dataset.bragg_peak,
    outofplane_angle=self.Dataset.outofplane_angle,
    inplane_angle=self.Dataset.inplane_angle,
    tilt_angle=self.Dataset.tilt_angle,
    # parameters when orthogonalizing the data before phasing
    # using xrayutilities
    sample_inplane=self.Dataset.sample_inplane,
    sample_outofplane=self.Dataset.sample_outofplane,
    offset_inplane=self.Dataset.offset_inplane,
    cch1=self.Dataset.cch1,
    cch2=self.Dataset.cch2,
    detrot=self.Dataset.detrot,
    tiltazimuth=self.Dataset.tiltazimuth,
    tilt_detector=self.Dataset.tilt_detector,
)

Running the functions

# Construct the argument parser and parse the command-line arguments
ap = argparse.ArgumentParser()
ap = add_cli_parameters(ap)
cli_args = vars(ap.parse_args())

# Load the config file
config_file = self.preprocessing_folder + "/config_preprocessing.yml"
parser = ConfigParser(config_file, cli_args)
args = parser.load_arguments()
args["time"] = f"{datetime.now()}"

# Run function
run_preprocessing(prm=args)
print("\nEnd of script")

[carnisj]

ok, then do you have unit tests for the function create_yaml_file? One should check what it is doing for the parameter "flip_reconstruction"

[carnisj]

I could reproduce the issue. The config file and the method create_yaml_file are fine. What happens is that a command line parameter is generated for "flip_reconstruction" with this long string value, and this overrides the correct boolean value that you generate in the config file (normal behavior).

On gwaihir side, we need therefore to understand why a command line parameter is generated and why only for "flip_reconstruction"

[carnisj]

Although you are not using any command line argument, flip_reconstruction is set to this value while other parameters are None as expected. This is beyond my understanding. I suggest that you remove the lines related to the command line parameters. I will allow it to be None by default, so that you can simply load the config_file and parse it as follows:

# Load the config file
config_file = self.preprocessing_folder + "/config_preprocessing.yml"
parser = ConfigParser(config_file)

[DSimonne]

Hi, That's rather odd, the command that is used to launch the script should not generate any command line parameter as seen above ...

[carnisj]

Same issue encountered by Ericmoore Jossou also on a Jupyter notebook. Interestingly, reducing the lenth of the parameter name by only one letter solves it, but I can't find anything related on the web.

-> change the parameter name from 'flip_reconstruction' to "flip_crystal"