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

Solver instantiation in operations should be done at the execute method #63

Open
jmhorcas opened this issue Aug 7, 2024 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@jmhorcas
Copy link
Contributor

jmhorcas commented Aug 7, 2024

In the current implementation, the Solver instance is created in the init of each analysis operation. After execute the operation, the solver is deleted as needed. However, it makes the operation totally useless for future calls if the operation itself is not instantiated again. Thus, this makes the framework not worthy to call multiple times an analysis operation.
To solve this, the Solver instance should be created at the "execute" method.
An alternative solution may be to "clean" the solver after executing the operation in a different way (not deleting it).
Ideally, IMHO the operation should be implemented within a "with" block to avoid all this inconvenients.

Example:
I want to call multiple times to the PySATSatisfiableConfiguration (e.g., to implement another operation such as "complete_partial_configuration"). Each call is over a different configuration (the same but we have added some features). Thus, I have to instante the operation itself multiple times degrading the performance of my new operation.

@jmhorcas jmhorcas added the enhancement New feature or request label Aug 7, 2024
@jagalindo
Copy link
Member

Yes, that was actually thougt that way. TTake into account that the default ussage of the tool (not the framework is by the following means

Command line usage
flamapy satisfiable "path/to/feature/model"

Python easy to use facade usage
from flamapy.interfaces.python.flamapy_feature_model import FLAMAFeatureModel

Load the feature model

fm = FLAMAFeatureModel("path/to/feature/model")

This method could be called with the param with_sat: bool = True if you want to force pysat (useful for WASM enviroments)

result = fm.satisfiable()
print(result)

However, it's trye that ehn using the framework, it might be interesting to maintain an instance of the solver to be reused. Nonetheless, in the past we had bad experiences if reusing the solver (sat4j). As when you apply multiple assumptions on them, it might return wrong results, so its safer to just clean it and pay off efficiency.

We can discuss this on the next meet up.

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