admb-project / admb

AD Model Builder
http://admb-project.org
Other
64 stars 19 forks source link

Improvements to the hess_step feature #200

Open Cole-Monnahan-NOAA opened 3 years ago

Cole-Monnahan-NOAA commented 3 years ago

An experimental feature was added in 12.3 (see https://github.com/admb-project/admb/issues/160). This issue is a place to report issues as it is tested.

So far I've found that

Regarding that last point, @wStockhausen and I investigated a bit and it appears that this may be related to at least dev_vector types. At least it seems to be more consistent with models without any bounded variables. The differences are typically very small but this feature works on very small values so it can be important. A more thorough investigation looking at initialization is needed.

Please add any comments/suggestions/requests related to this feature here. I will try to work on them before the next release.

johnoel commented 3 years ago

If the tests files that produce the errors are made available, it would be helpful for testing and debugging. I am especially curious of the last issue using bounded variables. Maybe, it is related to issue #76?

Cole-Monnahan-NOAA commented 2 years ago

I made some key updates to this functionality, resolving most of the issues presented here. Closing this now, we can reopen if I hear feedback from users in the future.

Cole-Monnahan-NOAA commented 1 year ago

Reopening this b/c some features are still broken:

Rick-Methot-NOAA commented 1 year ago

regarding dev_vectors: I have noticed that when a dev_vector is read into ADMB from a par file it gets re-normalized. One consequence is that if an element of that vector was near the bound it could get re-normalized to be outside the bound. I may have seen this happen during the BETWEEN_PHASES section also. Bringing this to Jon's attention. @johnoel

Cole-Monnahan-NOAA commented 1 year ago

Dave talks about this in the email thread about why they break for MCMC as well. Despite starting at the "wrong" mag, the hess_step feature still seems to work. It just may be disconcerting for the user to see the wrong mag in the output. I probably should put some kind of warning in there.

johnoel commented 1 year ago

Hi @Cole-Monnahan-NOAA, I can work with you on this to fix some of those issues. For dev_vector, there is a workaround, but I will need to do work on it.

Cole-Monnahan-NOAA commented 1 year ago

What is the work around? First step we should find some reprexes to demonstrate the issue. I have some stock assessments but simpler models would be better. @johnoel is there an easy way to loop through all the examples and run hess_step and check for failures?

johnoel commented 1 year ago

What are you trying to do? Please provide program arguments to one example.

Cole-Monnahan-NOAA commented 1 year ago

For instance go to the catage example and run

catage

You'll see the console output and the top of the catage.par file has that mag=8.74065e-05

now run catage -hess_step

and the console prints

Hess step 0: Max gradient=5.3001e-05

That does not match. So the initialization is slightly different. In other models this will be much more extreme.

Repeat with the 'simple' example and you'll see that these values will match. I think this is b/c catage uses bounded_dev_vector types.

Cole-Monnahan-NOAA commented 1 year ago

I just turned off estimation of the dev_vectors (set phase to negative) and now the two match

mag=3.94293e-05 Hess step 0: Max gradient=3.94293e-05

This further supports that there are issues with intitializing the dev_vectors.

johnoel commented 1 year ago

It's a known issue with the dev types. Dave suggested not using them. He mentioned that it's a case where two features created a bug. A possible workaround is replace the dev types with just regular admb variable types, then initialize the types in the initialization section. I haven't tested it, but it should reinitialize the type for MCMC.

Cole-Monnahan-NOAA commented 1 year ago

I'm a fan of not using them, but it's not as simple as just changing back to standard vector because there is a degree of freedom lost by enforcing the sum-to-zero constraint. Ideally we'd find a way for those dev_vectors to initialize exactly the same. Another possible approach is to throw a warning that if a dev_vector is used in the model that the user should not expect those mag's to agree. Is it possible to test whether there are active dev_vectors in a model?

johnoel commented 1 year ago

There is a way to code. Can you provide a simple example for me to work with?

Rick-Methot-NOAA commented 1 year ago

The existing catage.tpl example is a simple demo per COle's earlier comment

Rick-Methot-NOAA commented 1 year ago

Here is example showing consequences of the dev approach. image image

Cole-Monnahan-NOAA commented 1 year ago

@Rick-Methot-NOAA Very interesting and counterintuitive to me. I think it's important but perhaps a bit off topic here. Could you repost at SS3 repo or FIMS? I think this relates more to thinking about model parameterization.

Does the rest of the model expectations match?