search

NOAA-FIMS / FIMS

The repository for development of FIMS
https://noaa-fims.github.io/FIMS/
GNU General Public License v3.0
11 stars 7 forks source link

[Refactor]: Remove exit(1) from Information::CreateModel #550

Open msupernaw opened 3 months ago

msupernaw commented 3 months ago

Refactor request

If there is an error, CreateModel should only set valid_model to false and not call exit(1). We want to capture all errors that may occur in a single pass.

k-doering-NOAA commented 3 months ago

@msupernaw, is there a reason we don't want to fail fast?

nathanvaughan-NOAA commented 3 months ago

@Matthew Supernaw - NOAA Federal @.> That exit(1) was a request from @Cole Monnahan - NOAA Federal @.> I think during the logging review process. Don't really mind which way we fall on it but seems like it is really a preference vs objective best option. I would say that once you have one error you often has masses of role on errors that aren't real and are just carryover issues which may distract users if they see them all?



Nathan R. Vaughan Ph.D.
Research Scientist
Vaughan Analytics, supporting NOAA-Fisheries SEFSC
75 Virginia Beach Drive
Miami, FL 33149
Phone: (305) 733-6643
Email: ***@***.***
Alt email: ***@***.***

~~~<*)))))~< ~~~~<*)))))~< ~~~~<*)))))~< ~~~~

On Wed, Feb 28, 2024 at 1:30 PM Kathryn Doering ***@***.***>
wrote:

> @msupernaw <https://github.com/msupernaw>, is there a reason we don't
> want to fail fast
> <https://stackoverflow.com/questions/2807241/what-does-the-expression-fail-early-mean-and-when-would-you-want-to-do-so>
> ?
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/NOAA-FIMS/FIMS/issues/550#issuecomment-1969592884>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AMU2O2V272AHGZ7MCIZ4PNTYV5ZVZAVCNFSM6AAAAABD6REQT2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNRZGU4TEOBYGQ>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
Cole-Monnahan-NOAA commented 3 months ago

My request is that FIMS should not error out when calculating NaN values of the objective function or gradients. For both optimization and NUTS especially the algorithms will often try parameter combinations that lead to nonsensical values. The algorithms should handle those gracefully and not exit out.

See this discussion: https://github.com/admb-project/admb/issues/264

So it could be a global setting or something.

If there's an error building the model or something similar then this does not apply. It applies only to invalid objective calculations.

ChristineStawitz-NOAA commented 1 month ago

moving this to parking lot until we have more user feedback from the case studies