The argument "estimate_start" is not set to default after including helpers.py file

[Ameek-Sidhu]

I ran the "run_pahfit.py" script on the spectrum of M101 Nucleus without using the argument --estimate_start and the fit was unsuccessful with the message that the number of calls to the function has reached maxfev=1000.

The second time, I tested the "run_pahfit.py" script on the same spectrum with the argument --estimate_start in the command line and the fit was successful.

[Ameek-Sidhu]

@karllark, @els1 I think the following are the bugs in the code 1) In the function "initialize_model" in the helpers.py file, the argument estimate_start is set to False. Shouldn't this be set to "True". 2) In the same function "initialize_model" in the helpers.py file, while calling "PAHFITBase", the argument estimate_start is set as estimate_start = estimate_start. Since it's a boolean, shouldn't this be set to True. 3) In the file base.py the argument 'estimate_start' is set to False. Again shouldn't this be set to 'True'.

[Ameek-Sidhu]

I ran the code on my machine after making the changes (1,2, and 3 mentioned above) and the fit was successful without including the argument --estimate_start in the command line.

[els1]

Do you need 2?

[Ameek-Sidhu]

You are right @els1. It would work even if we don't include 1 as well as 2.

[Ameek-Sidhu]

I ran the "run_pahfit.py" script on the spectrum of M101 Nucleus without using the argument --estimate_start and the fit was unsuccessful with the message that the number of calls to the function has reached maxfev=1000.

The second time, I tested the "run_pahfit.py" script on the same spectrum with the argument --estimate_start in the command line and the fit was successful.

Just a correction, the fit is not successful with the argument --estimate_start, because we get the error message ('Both actual and predicted relative reductions in the sum of squares are at most 0.000000')

[karllark]

• In the function "initialize_model" in the helpers.py file, the argument estimate_start is set to False. Shouldn't this be set to "True".

initilize_model is used for other purposes than fitting, hence it is better to have the default be False. For example, when use plot_pahfit, we don't want/need to estimate start as the parameters are coming from the saved parameter file.

• In the same function "initialize_model" in the helpers.py file, while calling "PAHFITBase", the argument estimate_start is set as estimate_start = estimate_start. Since it's a boolean, shouldn't this be set to True.

Same answer as above.

• In the file base.py the argument 'estimate_start' is set to False. Again shouldn't this be set to 'True'.

Same answer as above.

To get the right behavior the fit_pahfit argparse parameter --estimate_start should be changed. This is the bug I think. I would suggest changing this parameter to be --no_starting_estimate and passing the opposite via not args.no_starting_estimate to initialize_model.

[karllark]

Just a correction, the fit is not successful with the argument --estimate_start, because we get the error message ('Both actual and predicted relative reductions in the sum of squares are at most 0.000000')

Was the fit not successful in that the residuals were bad? I'm wondering if this is just a log message from the fitting function that relates to why the fit stopped.

[alexmaragko]

To get the right behavior the fit_pahfit argparse parameter --estimate_start should be changed. This is the bug I think. I would suggest changing this parameter to be --no_starting_estimate and passing the opposite via not args.no_starting_estimate to initialize_model.

Looking currently into this. By fit_pahfit do you mean the run_pahfit script or the fit_spectrum function in helpers.py? Currently if you run run_pahfit with the --estimate_start argument the initialize_model function in helpers.py gets the proper boolean which is 'True'.

[karllark]

The idea I had was that if you run fit_pahfit the default should be to have estimate_start = True. Not quite sure how to get argparse to have the default as true. And I think it makes more sense to have the keyword to change the default behavior and and have a name that says what it will do.

[alexmaragko]

I assume by fit_pahfit you mean run_pahfit, right? When I was initially implementing the flag I was suggesting to have this by default True and set a --noguess flag (see #99 and #101), but If I recall correctly you mentioned you preferred the default to be False so we set the --estimate_start flag. Do we want revert this now, or I'm misinterpreting things. :)

[karllark]

Hah. You are right. We are reverting to what you originally suggests. Apologies - clearly I will catch up to your thinking at some point. ;-)

[alexmaragko]

No worries! I will be looking into this tonight in our meeting.

[karllark]

Fixed with PR #136