Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Continue running check_constraints_are_satisfied beyond first failure #436

Closed
hmgaudecker opened this issue Feb 11, 2023 · 2 comments
Closed
Labels
enhancement New feature or request

Comments

@hmgaudecker
Copy link
Member

  • estimagic version used, if any: 0.4.3

What would you like to enhance and why? Is it related to an issue/problem?

check_constraints seems to return only the first failure it encounters.

I have a pipeline that runs for a fairly long time until these types of failures are encountered and they are non-trivial to predict (skillmodels running 3 out of 4 factor models individually to get start parameters with cross-factor constraints), so debugging takes quite a bit of time.

Describe the solution you'd like

As usual with "test failures" (same kind of problem), it would be great to see all constraints that are violated at once.

I must admit that with the function name (check_constraints_are_satisfied) and docstring ("Raises: ValueError if constraints are not satisfied`) I was expecting this type of behaviour.

Describe alternatives you've considered

If the above proves difficult, at a minimum change the docstring to:

Raises:
    ValueError for the first constraint that is not satisfied
@hmgaudecker hmgaudecker added the enhancement New feature or request label Feb 11, 2023
@janosg
Copy link
Member

janosg commented Feb 13, 2023

Thanks for opening the issue.

In check_constraints_are_satisfied we can definitely do what you suggest. An EPP student is currently trying to make that code faster and I think improving the report could be a nice addition to the project (@Kaniyaki, we can discuss this at some point).

A good example of how to do it is this skillmodels code.

In other places it will be harder to keep running when something goes wrong. We currently raise very early so we can guarantee a reasonable error message and make more assumptions down the road, which reduces complexity in already very complex code.

But actually, I think your use-case is completely covered by these simple changes.

@janosg
Copy link
Member

janosg commented Jul 10, 2024

Solved by #450

@janosg janosg closed this as completed Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants