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

Implement global context #221

Closed
niklasdewally opened this issue Feb 14, 2024 · 11 comments
Closed

Implement global context #221

niklasdewally opened this issue Feb 14, 2024 · 11 comments
Assignees
Labels
area::conjure-oxide Related to conjure_oxide. area::logging-stats Related to logging and getting statistics out of Conjure Oxide kind::discussion General discussion and high-level planning. kind::feature New feature or request priority::next We need this next. status::blocked Waiting on some other features first.

Comments

@niklasdewally
Copy link
Collaborator

niklasdewally commented Feb 14, 2024

Problems

User settings

There is information given by the user at the start of the program that we will need to use all the way through the program.
For example:

  1. What solver type (Minion vs SAT)
  2. What solver implementation (eg for SAT, lingling or kissat.)
  3. For SAT, what type of encoding to use?

This information is useful both during rewriting, and to decide what solver to use / what parameters to give it.
See: #199 #103

Solver stats

Solvers can produce different metadata / stats, such as CPU time or node count.
Conjure now can capture this and store a s a JSON file. We need to be able to extract this information and use it in our CI (for performance testing) - see #247 #248

Having a general way to pass information between layers is useful in general

Proposal

TODO

Questions

  1. What options do we need?
  2. How will we store solver specific information?
@niklasdewally
Copy link
Collaborator Author

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Feb 14, 2024

For clarification, solver type vs implementation is an important distinction because we do not care in most places that we are using Lingling, Caddical, etc, just that we are solving for SAT. Solver type will be used to determine which rules apply, and the Solver Implementation will be used to determine what solver adaptor to use.

@niklasdewally niklasdewally added kind::feature New feature or request area::conjure-oxide Related to conjure_oxide. labels Feb 14, 2024
@niklasdewally niklasdewally added priority::soon We'll need this soon, or other features may be blocked. status::vip-help-wanted This issue needs someone to work on it. labels Feb 20, 2024
@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Feb 22, 2024

(FYI @gskorokhod @lixitrixi)

Some additional requirements discussed in yesterdays meeting:

Configuration needs to be thread-safe - i.e. usable across threads. We also may want multiple configurations at once - for example, so that cargo test can run multiple runs of the rewriter in parallel using different settings. Therefore, configuration can not be a global variable.

Some other (optional) thoughts:

  • A nice pattern to avoid having pass-through variables in every function is to have the main data-structure in each layer of abstraction hold a pointer to its "context" (Ousterhout, Software Design). In our case, this could mean that the "rewriting" layer could access global context through a pointer in the Model struct and the "solving" layer could access global context through a pointer in the Solver struct. This would mean basically no refactoring apart from at the boundaries between the parse and rewriting and rewriting and solver layers. In Rust this pointer would probably be an Arc<Context>.

  • The Context could be immutable with us allowing the mutation of specific inner values only through a Mutex<>.

  • To ensure backwards compatibility as we add stuff to it, contexts could be unable to be constructed manually, using a builder and/or ::new().

@niklasdewally
Copy link
Collaborator Author

Happy for you to take this @gskorokhod if you are still interested :)

@gskorokhod gskorokhod added the kind::discussion General discussion and high-level planning. label Feb 22, 2024
@gskorokhod
Copy link
Contributor

Happy for you to take this @gskorokhod if you are still interested :)

Hmm, I could! I will take a few days to get my midterm report submited and start working on the next C practical though

@gskorokhod
Copy link
Contributor

gskorokhod commented Feb 22, 2024

I think i'm interested in working on uniplate and learning more about that though. But I'm still happy to take this if need be!

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Feb 22, 2024

I think i'm interested in working on uniplate and learning more about that though. But I'm still happy to take this if need be!

Just brought it up because you mentioned it earlier :) It's marked as help wanted so someone might pick it up at some point. Otherwise I imagine Felix or me may end up doing this as we need it for SAT.

@niklasdewally niklasdewally added the priority::next We need this next. label Mar 15, 2024
@gskorokhod gskorokhod removed the priority::soon We'll need this soon, or other features may be blocked. label Mar 15, 2024
@niklasdewally niklasdewally added status::blocked Waiting on some other features first. and removed priority::soon We'll need this soon, or other features may be blocked. labels Mar 15, 2024
@gskorokhod gskorokhod self-assigned this Mar 15, 2024
@gskorokhod gskorokhod changed the title Storing Global Configuration, and using this for rule selection and solver selection Implement global context Mar 15, 2024
@gskorokhod
Copy link
Contributor

Reanimating this issue because we need it for performance testing :)

@niklasdewally niklasdewally removed the status::vip-help-wanted This issue needs someone to work on it. label Mar 15, 2024
@gskorokhod
Copy link
Contributor

From Friday meeting:

  • Our system is multi-threaded, so every thread should have its own global context (hence it can't be a static object)
  • Context should be a part of the object that gets passed around between layers (e.g. Model). not a separate parameter
  • Every layer should be self contained. In the future, it should be possible to support different use cases, such as:
    • Importing conjure as a library and running each layer individually from within a user's Rust program
    • Running conjure as a server, taking new problems as they come, and saving something / doing some computation in between (e.g. caching parts of the rewrite to speed things up)

@niklasdewally niklasdewally added the area::logging-stats Related to logging and getting statistics out of Conjure Oxide label Oct 2, 2024
@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Oct 2, 2024

@ozgurakgun @gskorokhod what's the current status of this? I think this issue has been completed.

@niklasdewally
Copy link
Collaborator Author

niklasdewally commented Nov 12, 2024

@ozgurakgun closing this as completed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area::conjure-oxide Related to conjure_oxide. area::logging-stats Related to logging and getting statistics out of Conjure Oxide kind::discussion General discussion and high-level planning. kind::feature New feature or request priority::next We need this next. status::blocked Waiting on some other features first.
Projects
None yet
Development

No branches or pull requests

2 participants