Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix indices for optimization of sized parameters with size==1 #68

Merged
merged 2 commits into from Nov 2, 2023

Conversation

schmoelder
Copy link
Contributor

@schmoelder schmoelder commented Nov 2, 2023

An issue was reported on the forum where a user tried adding an optimization variable for a sized parameter with a single entry (e.g. when n_comp == 1) and an error was raised.

E.g.

optimization_problem = OptimizationProblem('fit_param', use_diskcache=False)

optimization_problem.add_evaluation_object(process)

optimization_problem.add_variable(
    name='film_diffusion', 
    parameter_path='flow_sheet.column.film_diffusion',
    indices=0,      # <-- I've tried with and without this
    lb=1e-10, ub=1.0,
    transform='auto'
)

optimization_problem.add_variable(
    name='axial_dispersion', 
    parameter_path='flow_sheet.column.axial_dispersion',
    lb=1e-10, ub=0.1, transform='auto'
)

Raises:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[14], line 5
      1 optimization_problem = OptimizationProblem('fit_param', use_diskcache=False)
      3 optimization_problem.add_evaluation_object(process)
----> 5 optimization_problem.add_variable(
      6     name='film_diffusion', 
      7     parameter_path='flow_sheet.column.film_diffusion',
      8     indices=0, 
      9     lb=1e-10, ub=1.0,
     10     transform='auto'
     11 )
     13 optimization_problem.add_variable(
     14     name='axial_dispersion', 
     15     parameter_path='flow_sheet.column.axial_dispersion',
     16     lb=1e-10, ub=0.1, transform='auto'
     17 )
     19 # optimization_problem.add_variable(
     20 #     name='isotherm_constant', 
     21 #     parameter_path='flow_sheet.column.binding_model.adsorption_rate',
     22 #     lb=1e-2, ub=10.0,
     23 #     transform='auto'
     24 # )

File ~\miniconda3\envs\cadet\lib\site-packages\CADETProcess\optimization\optimizationProblem.py:347, in OptimizationProblem.add_variable(self, name, evaluation_objects, parameter_path, lb, ub, transform, indices)
    342 if parameter_path is not None and len(evaluation_objects) == 0:
    343     raise ValueError(
    344         "Cannot set parameter_path for variable without evaluation object "
    345     )
--> 347 var = OptimizationVariable(
    348     name, evaluation_objects, parameter_path,
    349     lb=lb, ub=ub, transform=transform,
    350     indices=indices,
    351 )
    353 self._variables.append(var)
    355 with warnings.catch_warnings():

File ~\miniconda3\envs\cadet\lib\site-packages\CADETProcess\optimization\optimizationProblem.py:2962, in OptimizationVariable.__init__(self, name, evaluation_objects, parameter_path, lb, ub, transform, indices, precision)
   2960     self.evaluation_objects = evaluation_objects
   2961     self.parameter_path = parameter_path
-> 2962     self.indices = indices
   2963 else:
   2964     self.evaluation_objects = None

File ~\miniconda3\envs\cadet\lib\site-packages\CADETProcess\optimization\optimizationProblem.py:3169, in OptimizationVariable.indices(self, indices)
   3167     _ = self.indices
   3168 except (ValueError, TypeError) as e:
-> 3169     raise e

File ~\miniconda3\envs\cadet\lib\site-packages\CADETProcess\optimization\optimizationProblem.py:3167, in OptimizationVariable.indices(self, indices)
   3165 # Since indices are constructed on `get`, call the property here:
   3166 try:
-> 3167     _ = self.indices
   3168 except (ValueError, TypeError) as e:
   3169     raise e

File ~\miniconda3\envs\cadet\lib\site-packages\CADETProcess\optimization\optimizationProblem.py:3126, in OptimizationVariable.indices(self)
   3124 parameter_shape = self._get_parameter_shape(eval_obj)
   3125 if isinstance(parameter_shape, tuple):
-> 3126     eval_ind = generate_indices(parameter_shape, eval_ind)
   3127 else:
   3128     if not isinstance(eval_ind, list):

File ~\miniconda3\envs\cadet\lib\site-packages\CADETProcess\dynamicEvents\section.py:663, in generate_indices(shape, indices)
    661 size = np.prod(shape)
    662 if size == 1:
--> 663     raise ValueError("Scalar parameters cannot have index slices.")
    665 if indices is None:
    666     indices = np.s_[:]

ValueError: Scalar parameters cannot have index slices.

This PR addresses this issue. For this purpose, @ronald-jaepel created a hotfix which simply removes a check. While this works, it might have unforeseen consequences.

The function generate_indices is only used internally and generally, size is checked before calling it. However, I'll have to setup some tests to verify this.

@schmoelder schmoelder merged commit e6b4623 into master Nov 2, 2023
6 checks passed
@Stefanos1100
Copy link

Hello, I have the same problem.
Could you please explain again what I need to do to solve it? The previous answers are unfortunately unclear to me.

@ronald-jaepel
Copy link
Collaborator

Hello Stefanos,

can you please share a minimum reproducible example? I can currently not produce the error. Please also open a new issue, as this one is closed.

@schmoelder schmoelder deleted the fix/scalar_parameter_index branch January 10, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants