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

Refactor lc_solve_problem #584

Open
janosg opened this issue May 24, 2024 · 0 comments
Open

Refactor lc_solve_problem #584

janosg opened this issue May 24, 2024 · 0 comments

Comments

@janosg
Copy link
Collaborator

janosg commented May 24, 2024

We should refactor pydvl.valuation.methods._solve_least_core_problems.lc_solve_problem

In the discussion of PR #580, @schroedk wrote:

This function is mixing two levels of abstraction, on the one hand it is doing a lot of low level stuff to construct the conditions for the optimization problem and on the other hand, it calls other more low-level functions (solve...). This mixing of levels of abstractions makes it harder to read. Even if a user would not call this function directly, for the developers it is a more difficult task to grasp what is happening (actually the comment helps a lot and in combination with the log messages it seems to me that the related code should be encapsulated in other low-level functions with only one purpose).

Another problem is that all low level functions that are called inside lc_solve_problem have many similar argument (e.g. A_eq, b_eq, A_lb, b_lb). We should bundle those arguments in a simple Object. That way we can pass them around more easily and do some simple compatibility checks during initialization.

@janosg janosg mentioned this issue May 24, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant