thouska / spotpy

A Statistical Parameter Optimization Tool
https://spotpy.readthedocs.io/en/latest/
MIT License
254 stars 152 forks source link

Premature stop if spotpy.parameter.Constant used in SCEUA #175

Closed Fratorhe closed 6 years ago

Fratorhe commented 6 years ago

Hello,

I have a large set of parameters and I want to keep some constant while I optimize for some others. However, when I put those parameters as spotpy.parameter.Constant, the optimization prematurely stops when "Computes the normalized geometric range of the parameters", since dividing by bound (which will be 0 in the case of constant) will give as results NaNs.

Thanks for your time, Francisco

thouska commented 6 years ago

Hi @Fratorhe, thank you for your message. I can conforim, this is unfortunatelly a bug and needs to be fixed. I will look into this in the near future. For now you may not want to use the constant parameters and set them in the def simulation() function. This should enable you to run SCE-UA as intended. I will inform you, as soon as I fixed the issue. Just for the recorder: This is issue is most likely also affecting the algorithms abc, fscabcand dream.

Fratorhe commented 6 years ago

Hi @thouska , Thanks for your quick response. Indeed, I am setting their value in the def simulation and it works great. It'd just be more handy to have the constant option so it's easier to test different configurations without having to touch the model and the vector of unknowns. Thanks for your time

philippkraft commented 6 years ago

Should be solved in the next release.

@Fratorhe If you like to try it out earlier, please get spotpy from PR #179 and test if your old version is running now.

Fratorhe commented 6 years ago

Hello,

I have uninstalled the spotpy with pip and installed this version with python setup.py install

Now, when I try to run it with the PR #179 https://github.com/thouska/spotpy/pull/179, I receive the following error message: File "C:\ProgramData\Anaconda3\lib\site-packages\spotpy-1.3.30-py3.6.egg\spotpy\algorithms_algorithm.py", line 312, in postprocessing

12 is the number of unknowns of my problem, and if I check the self.par_positions it gives an empty list. Could you give me a hand?

Best, Francisco

On Wed, Sep 5, 2018 at 9:27 AM Philipp Kraft notifications@github.com wrote:

Should be solved in the next release.

@Fratorhe https://github.com/Fratorhe If you like to try it out earlier, please get spotpy from PR #179 https://github.com/thouska/spotpy/pull/179 and test if your old version is running now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/thouska/spotpy/issues/175#issuecomment-418794512, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NVRYM_J40Wl__LVp8wmj8TyMomtEnks5uX_uKgaJpZM4WQaeX .

philippkraft commented 6 years ago

@Fratorhe: Sorry, in between thouska and me had another idea to solve the problem, to prevent future problems - but this is not working by now. Currently the PR is broken. Could you try again as soon as the PR has a successful build on travis? This might take a couple of days. However, if you like to help us, maybe you can create a toy version of your use case (same parameters, simple and fast equation, eg. Rosenbrock) and write it as a test case?

Fratorhe commented 6 years ago

Hello,

Thanks for your quick response. Please find attached a dummy case with the capability I would like spotpy to support. Basically, it's being able to alternate between line 30 and 31. In the optimization cases that I am studying, it is interesting sometimes to keep some parameters to a constant while optimizing the others. This fix would allow to not have to change the vector everytime I want to set a parameter to a constant value. For the time being, I give narrow values to the bounds in a uniform distribution, but having the constant capability would be more clear and (in my opinion) more user friendly.

Thanks again for your time, Francisco

On Fri, Sep 7, 2018 at 12:02 AM Philipp Kraft notifications@github.com wrote:

@Fratorhe https://github.com/Fratorhe: Sorry, in between thouska and me had another idea to solve the problem, to prevent future problems - but this is not working by now. Currently the PR is broken. Could you try again as soon as the PR has a successful build on travis? This might take a couple of days. However, if you like to help us, maybe you can create a toy version of your use case (same parameters, simple and fast equation, eg. Rosenbrock) and write it as a test case?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/thouska/spotpy/issues/175#issuecomment-419343255, or mute the thread https://github.com/notifications/unsubscribe-auth/AH4NVZ-bCDFxTPdVL_4FTu6ledeEI1NHks5uYhnogaJpZM4WQaeX .

philippkraft commented 6 years ago

Dear Francisco,

There is no attached code in your comment. Can you post it eg. as a gist or share it some other way? Or if you can have a look at

https://github.com/philippkraft/spotpy/blob/995fe6040a2c6279d078b162a173afe776a3fd93/spotpy/unittests/test_list_and_constant_parameters_with_samplers.py#L15-L51

And see if your case is covered already?

Fratorhe commented 6 years ago

Sorry, I attached it in the email... Let's see if this works: https://gist.github.com/Fratorhe/076f24a98aa8be2c7aefbfabcb46cca0

Thanks again for your time

thouska commented 6 years ago

Hi @Fratorhe thanks for this test script. I started it with the changes we made in #179 and got no errros. The Constant parameters stay constant now. Technically this was solved by testing in parameters.py if a Constant parameter is used anywhere in the user defined spot_setup class. If so, _algorithm.py takes care that they are not altered in any of the algorithms. We could also think of testing, if parameter['maxbound'] - paramter['mibound'] == 0. Benefit: more robust for useres that generate constant behaviour with parameter.uniform(low=high), which only captured in sceua yet. But this would also filter List parameters. So I would say we go with the current version merged in #179 for now. Thank you for informing us about this bug.