iMinuit minimizer

[ggalloni]

Hello, I added iMinuit as an alternative to scipy.optimize and py-BOBYQA. Indeed, iMinuit has a scipy-like interface that allows to essentially copy how the scipy minimization is already implemented in cobaya. In other words, the modifications needed are minimal.

iMinuit has already been used in combination with cobaya in the literature and also multiple times in a cosmological framework, for example within the Planck Collaboration. A recent example is 2205.05617, which reported significantly better performances using iMinuit w.r.t. the other two minimizers.

Alongside the addition of iMinuit, I added an attribute to the Minimize object collecting the complete array of minima found during a minimize run with multiple initial points. In my opinion, this is useful to have on the user-side since the dispersion of the minima can give an insight on the accuracy of the minimization itself. The same quantity is also passed on to the method that extracts the products resulting from minimization.

Running pytest on test_minimize.py produces no errors.

[cmbant]

Thanks! This looks good except that it assumes iminuit is installed without making it a requirement. I'm not sure we want to make it a cobaya requirement, so I would import it dynamically and raise an appropriate error as needed. It would also be a good idea to add a short fast test to tests/test_minimize (fine to add iminuit to install requires).

[ggalloni]

Oh yeah, I forgot about that, sorry. I removed iminuit from the global imports and instead added a try-except statement to import it or raise a LoggedError. Of course, the statement is compiled only when iminuit is requested by the user, thus inside the if statement of the run method. Let me know if this is what you had in mind, otherwise I'll modify the script as you prefer. For example, another possibility would be to catch the ImportError in the overall try-except statement that for now is just handling unexpected errors (lines 333 to 335), but I am not sure I would mix the two.

Instead, I didn't get what test you would add to test_minimize. As of now that script should already test the iMinuit routine since it is part of the "valid_methods" tuple defined at the top of minimize.py, right?

Anyway, thank you for the very fast response! :)

[cmbant]

Ah right, if the test passes then that's fine then. You need to add iminuit to the tmp's "test" optional dependencies.

[cmbant]

I'm not sure why these tests are failing, there's nothing obvious you've changed that looks immediately problematic. However, my own recent PR updates have completed tests just fine, so it's also not obviously travis playing up

[ggalloni]

I am testing an idea now: some time ago I had some unexpected problems with the deepcopy in the log.py file. It seems that when that tries to deepcopy the Minuit instance it goes on forever meeting the Python recursion limit. Unfortunately, it is not possible to import any alternative from tools.py since that causes a cyclic import, so I am trying a workaround that worked in the past on my local machine. If this fixes it we can think of a cleaner way to do this, otherwise I will revert the change.

[ggalloni]

Nope, it gets stalled in the test_cosmo_run.py again. I'm reverting the change, but still, I don't see how changing the minimizer would affect that...

[cmbant]

It seems to be hanging in the mpi tests, not sure why though

[ggalloni]

Mmh that's weird, how should we proceed? If you want I can comment everything I changed so that we can try and isolate the modification that is causing the problem.

[cmbant]

For some reason the last change didn't trigger test - I assume commenting out iminuit would fix it, but indeed good to checlk.

[ggalloni]

Yeah, it is still not triggering... It was doing the same on the first 4 commits here, while after your first response it started going through the checks for some reason. Do you think that I have to do something to my fork?

[ggalloni]

Ok, I built Travis on my fork and it is correctly triggering at each PR. I can reproduce the stall we were seeing here, so I am trying to debug it there.

[cmbant]

The travis requests say the below, not idea why permission seems to have changed, or why the "not parse" message:

master f0dfc65

27 minutes ago Workaround deepcopy

Sender ggalloni is not allowed to trigger build in this repo.

master 36bd983

38 minutes ago Test iminuit

Sender ggalloni is not allowed to trigger build in this repo.

master b4d5ea9

2 hours ago Test Travis

Could not parse

[cmbant]

I didn't get exactly was the issue with deepcopy in the log.py (i.e. where is a HasLogger instance copying something minuit-dependent?) f it is just the result dictionary sharing by mpi causing an issue, you can just pop "minuit" from the result dictionary before sharing with zip_gather in run()?

[cmbant]

It does seem to then work: https://github.com/CobayaSampler/cobaya/commit/2fdd091b7f1e9e36a643794c2c87d83e32e330f6

[ggalloni]

That's great! I'm giving it a try

[ggalloni]

Great, on my fork Travis passed every check! With the previous workaround it was breaking on Python 3.9 and 3.10, but of course, your solution solves the issue at its root.

For now, I'll leave the full set of minima out of the picture, cause I was finding some extra problems. I will try to solve it on my fork without spamming here. At some point, I will propose another PR if I solve everything.

Still, Travis here is not triggering (and I don't know if I can do something about it), but let me thank you very much for your support! :)

[cmbant]

Great, thanks for the PR!