Gpufit icon indicating copy to clipboard operation
Gpufit copied to clipboard

Different constraints for every fit

Open SebastianHambura opened this issue 3 years ago • 1 comments

Hi,

How are the constraints used for gpufit_constrained() ? In the documentation, it says the constraints array should be of size n_fits * n_parameters * 2, which indicates each constraint set is independent for each fit.

However, I tried using different constraints for each fit, and it seems like only the first set of constraints is used for every fit - which is not what I want. I've looked into the code to try to understand what happens, and I believe the faulty code is this (cuda_kernels.cu line 939) : https://github.com/gpufit/Gpufit/blob/c8026265b10e3fd00877ff4b06504ef048f59fe5/Gpufit/cuda_kernels.cu#L939-L946 As is, only the first constraints are used for every fit ( 0 <= parameters_to_fit_indices < n_parameters).

To fix this and be able to use different constraints for each fit, I believe the fix should look something like that (though I haven't tested it and I don't know enough of the code to be sure it's correct) :

    REAL & parameter = parameters[fit_index * n_parameters + parameters_to_fit_indices[parameter_index]];

    REAL const  & lower_bound = constraints[(fit_index * n_parameters + parameters_to_fit_indices[parameter_index]) * 2 + LOWER_BOUND];
    REAL const  & upper_bound = constraints[(fit_index * n_parameters + parameters_to_fit_indices[parameter_index]) * 2 + UPPER_BOUND];
    
    int const & constraint_type = constraint_types[parameters_to_fit_indices[parameter_index]];

    project_parameter_to_box(parameter, lower_bound, upper_bound, constraint_type);

In the Matlab example gauss2d_constrained, the constraints are the same for every fit, so the problem doesn't arise. Another way to solve this is just to say in the documentation that the same constraints apply for every fit (and change the size of the constraints array to n_parameters * 2)

I don't really need this functionality at the moment, but I though I would let you know :)

SebastianHambura avatar Apr 22 '22 08:04 SebastianHambura

This looks like a bug. Thanks for the feedback.

superchromix avatar Apr 22 '22 11:04 superchromix

@superchromix @SebastianHambura I've create a Pull Request for this issue, here: PR https://github.com/gpufit/Gpufit/pull/117. I haven't done much testing, but it builds.

jimkring avatar Oct 18 '22 06:10 jimkring

I tested @SebastianHambura 's proposed fix (cuda_kernels.cu line 939), and I think there is some additional code that needs to be modified, but I haven't gotten it right yet. In gpu_data.cu line 144 Currently IS: if (info_.use_constraints_) { write(constraints_, constraints, 2 * info_.n_parameters_); write(constraint_types_, constraint_types, info_.n_parameters_); }

Possible fix (this and minor variations didn't work, but I think it's in the ballpark): if (info_.use_constraints_) { write( constraints_, constraints + chunk_index_*info_.max_chunk_size_*info_.n_parameters_, chunk_size * info_.n_parameters_ * 2); write(constraint_types_, constraint_types, info_.n_parameters_); }

henrysillin avatar Oct 18 '22 23:10 henrysillin

@henrysillin I made your proposed change in PR https://github.com/gpufit/Gpufit/pull/117 (Note: I changed chunk_size to chunk_size_ in your possible fix). Still need to do some testing to see if it works on GPU.

jimkring avatar Oct 19 '22 02:10 jimkring

Fixed in c65c867b4528ddbf3ed6af6428f589fb93fb972d. Thanks @SebastianHambura @henrysillin @jimkring.

gpufit avatar Oct 20 '22 19:10 gpufit

Looks good to me!

Here are two different fits that show it's now working.

We can see that the resulting fit parameters (outputs) for each curve fall within their own fit constraints and also (this is the good part) the 2nd curve's last fit parameter (8.1) falls below the constraints of the first fit (9.0 to 10.0).  Previously, it would get stuck at 9.0 (the lower end of the constraint for the first fit.

Kudos and thanks!

jimkring avatar Oct 20 '22 21:10 jimkring