Qcodes icon indicating copy to clipboard operation
Qcodes copied to clipboard

doNd: Integers are converted to floats

Open mgunyho opened this issue 3 years ago • 3 comments

Steps to reproduce

  1. Have two parameters, one that has float values, and one that has validator Ints
  2. Try to do a 2D sweep of the two parameters using dond

Expected behaviour

The sweep should work normally

Actual behaviour

TypeError, because dond is trying to set the integer parameter to a float value.

Minimal working example

import qcodes as qc
from qcodes.utils.validators import Ints
from qcodes.utils.dataset.doNd import dond, ArraySweep

qc.config.core.db_location = "./test.db"

qc.load_or_create_experiment("test")

float_param = qc.ManualParameter("float_param")
int_param = qc.ManualParameter("int_param", vals=Ints(0, 100))

dond(
    ArraySweep(float_param, [1.1, 2.2, 3.3]),  # if I comment out this line, works OK
    ArraySweep(int_param, [1, 2, 3]),
) # -> TypeError: ('1.0 is not an int; Parameter: int_param', 'setting int_param to 1.0')

Tested on qcodes v0.33.

mgunyho avatar Jul 06 '22 13:07 mgunyho

@mgunyho would it be a decent workaround for you to pass a numpy array of ints to ArraySweep as opposed to list of ints? the challange is that whatever you pass gets transofmed into a numpy array, and i guess list of ints also gets transformed to an array of floats.

astafan8 avatar Jul 06 '22 13:07 astafan8

The same happens if I pass it as a numpy array (either np.array([1,2,3]) or more explicitly np.array([1,2,3], dtype=np.int)). It also doesn't matter whether the float or int array is first. I didn't check what dond does internally but seems like it's coercing all of the arrays to the same datatype. Maybe using a pandas dataframe (or separate arrays for each param value) would work better with heterogeneous types.

For now my workaround was to just do a for-loop over the int-value (so one measurement for each) because I didn't have that many of them, but that's not very good in the long run.

mgunyho avatar Jul 06 '22 13:07 mgunyho

I think the issue is that do_nd bundles all the setpoints into one array (look at the implementation of _flatten_setpoint_values I will see if I can have a look at that as part of #4325

jenshnielsen avatar Jul 06 '22 13:07 jenshnielsen

@mgunyho I have added a test to https://github.com/QCoDeS/Qcodes/pull/4325 based on your original issue confirming that this has been resolved by the refactor in that pr. I have also clarified the types of the setpoints in LinSweep clarifying that this can only generate float setpoints

jenshnielsen avatar Sep 23 '22 11:09 jenshnielsen

So the test is now

def test_sweep_int_vs_float():

    float_param = ManualParameter("float_param", initial_value=0.0)
    int_param = ManualParameter("int_param", vals=Ints(0, 100))

    dond(ArraySweep(int_param, [1, 2, 3]), float_param)

I actually had something slightly different in mind with this issue. My problem originally was that I was trying to do a 2D sweep where one of the axes is an integer, and another one is float. The MWE is missing the parameter to be measured, this is what it should have been:

import qcodes as qc
from qcodes.utils.validators import Ints
from qcodes.utils.dataset.doNd import dond, ArraySweep

qc.config.core.db_location = "./test.db"

qc.load_or_create_experiment("test")

float_param = qc.ManualParameter("float_param")
int_param = qc.ManualParameter("int_param", vals=Ints(0, 100))
acquisition_param = qc.ManualParameter("thing_to_be_measured", initial_value=123)  # the parameter we're going to measure

dond(
    ArraySweep(float_param, [1.1, 2.2, 3.3]),
    ArraySweep(int_param, [1, 2, 3]),
    acquisition_param,  # this was missing
) # -> ValueError: 1.0 is not an int

But in any case, the issue is indeed fixed by the refactor.

While testing this, I noticed another bug: the original MWE, without the acquisition_param, where I accidentally forgot to add the parameter to be acquired causes an UnboundLocalError:

UnboundLocalError                         Traceback (most recent call last)
.../arraysweep_int_test.py in <module>
     14
     15 
---> 16 dond(
     17     ArraySweep(float_param, [1.1, 2.2, 3.3]),  # if I comment out this, works OK
     18     ArraySweep(int_param, [1, 2, 3]),

.../Qcodes/qcodes/dataset/dond/do_nd.py in dond(write_period, measurement_name, exp, enter_actions, exit_actions, do_plot, show_progress, use_threads, additional_setpoints, log_info, break_condition, dataset_dependencies, *params)
    552     sweeper = _Sweeper(sweep_instances, additional_setpoints)
    553 
--> 554     measurements = _Measurements(measurement_name, params_meas)
    555 
    556     LOG.info(

.../Qcodes/qcodes/dataset/dond/do_nd.py in __init__(self, measurement_name, params_meas)
    218             self._grouped_parameters,
    219             self._measured_parameters,
--> 220         ) = self._extract_parameters_by_type_and_group(params_meas)
    221         self.measurement_names = self._create_measurement_names(measurement_name)
    222 

.../Qcodes/qcodes/dataset/dond/do_nd.py in _extract_parameters_by_type_and_group(params_meas)
    274         if multi_group:
    275             grouped_parameters = tuple(multi_group)
--> 276         return tuple(measured_all), grouped_parameters, tuple(measured_parameters)
    277 
    278 

UnboundLocalError: local variable 'grouped_parameters' referenced before assignment

I suppose if the *args are only Sweep:s, and none are parameters, there should be an error message indicating that the measurement parameters are missing. (This is on version 0f54cf2f15)

mgunyho avatar Sep 23 '22 13:09 mgunyho

To clarify, here's a failing test for this:

def test_dond_without_measurement_params():
    a = ManualParameter("a", initial_value=0)

    with pytest.raises(
        ValueError
    ):
        dond(ArraySweep(a, [1, 2, 3]))

mgunyho avatar Sep 23 '22 13:09 mgunyho

Thanks I agree that we should give a better error message when there is no parameter to measure.

jenshnielsen avatar Sep 23 '22 14:09 jenshnielsen