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

Improve API ergonomics #82

Merged
merged 12 commits into from
Apr 15, 2024
Merged

Improve API ergonomics #82

merged 12 commits into from
Apr 15, 2024

Conversation

chrjabs
Copy link
Owner

@chrjabs chrjabs commented Apr 7, 2024

resolves #81

  • Add add_clause_ref and add_cnf_ref to Solve trait
  • Rename file writing methods to be write_ and make them take by reference. If they require the instance to be in a certain format, they will return an error if this is not the case.
  • Add _ref and _mut accessor methods to instance types and deprecate older accessors.
  • Add write_dimacs to Cnf
  • Add n_vars to SatInstance
  • Add add_nary to SatInstance and Cnf
  • Rename all "heavy" converter methods (taking by value) to into_ and all inplace converters to convert_to_

@chrjabs
Copy link
Owner Author

chrjabs commented Apr 10, 2024

Some more To-dos for this:

  • Instead of taking mutable references for to_dimacs etc, return an error if the instance is not in the required format
  • Name as_cnf, into_cnf following standard naming convention: converters (taking by value) are named into_
  • Name inplace converters convert_to_
  • File parsers that need BufRead should take a buffered reader as an input rather than wrapping a reader internally to avoid buffering twice
  • Implement Extend<&Clause> for solvers
  • Check whether some clones in tests can be avoided by add_clause_ref

Introduces:
- `SatInstance::var_manager_ref()`
- `SatInstance::var_manager_mut()`
- `SatInstance::n_vars()`
- `OptInstance::constraints_ref()`
- `OptInstance::constraints_mut()`
- `OptInstance::objective_ref()`
- `OptInstance::objective_mut()`
- `MultiOptInstance::constraints_ref()`
- `MultiOptInstance::constraints_mut()`
- `MultiOptInstance::objective_ref()`
- `MultiOptInstance::objective_mut()`

Deprecates:
- `SatInstance::var_manager()`
- `OptInstance::get_constraints()`
- `OptInstance::get_objective()`
- `MultiOptInstance::get_constraints()`
- `MultiOptInstance::get_objective()`
Make `to_opb` and similar opb writing functions take by reference
instead of by value. Some still need to take mutable references, provide
workarounds in documentation if additional guarantees are known.
Make `to_dimacs` and similar dimacs writing functions take by reference
instead of by value. For the high-level instance types mutable
references are still needed; provide `Cnf::to_dimacs` and workarounds in
documentation if additional guarantees are known.
Since external libraries cannot easily take ownership anyway, for most
solvers this is more reasonable.
Add a default implementation for `add_clause` that can be overwritten
for solvers that can truly take ownership.
@chrjabs chrjabs force-pushed the feature/ergonomics branch 2 times, most recently from f5921f4 to f812aec Compare April 15, 2024 12:32
- rename file writing methods `write_` rather than `to_`
- make all file writing methods take references and return errors if not
  in right format
- rename all heavy converters `into_`
- rename all inplace converters `convert_to_`
in tests and two methods of `Objective` type
@chrjabs chrjabs merged commit ed615f8 into main Apr 15, 2024
44 checks passed
@chrjabs chrjabs deleted the feature/ergonomics branch April 15, 2024 13:11
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

Successfully merging this pull request may close these issues.

General API ergonomics improvements
1 participant