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

Removing unused files, dead code. #157

Open
58 of 66 tasks
MichaelRawson opened this issue Oct 28, 2020 · 12 comments
Open
58 of 66 tasks

Removing unused files, dead code. #157

MichaelRawson opened this issue Oct 28, 2020 · 12 comments

Comments

@MichaelRawson
Copy link
Contributor

MichaelRawson commented Oct 28, 2020

Some code in Vampire's tree appears to no longer be used. While this is not a problem in itself, it adds to the noise for new contributors and some dead code is still being compiled or even linked (see e.g. PR #153 to remove Lingeling sources). Additionally, changes to interfaces require more effort as code that is no longer used needs fixing.

After some discussion with others, I believe some of the following could be removed:

  • Saturation/Limits are empty files.
  • Shell/SimplifyProver isn't used anywhere - used to be support for the Simplify theorem prover's language
  • similar but not the same, the mysterious Shell/ProofSimplifier
  • BFNT and BFNTMainLoop is an old form of finite model building.
  • Inferences/InterpretedSimplifier commented out
  • Indexing/ArithmeticIndex commented out
  • VUtil and friends - no longer build, not touched in several years
  • various "frontends":
    • vutil
    • ucompit
    • vclausify
    • vcompit
    • vground
    • vsat
    • vltb
    • vtest
  • Significant chunk of Test
  • value in TimeCounterUnit: TC_BDD etc.
  • SMTLIB 1 support?
  • TWLSolver
  • Lib::InPlaceStack
  • Many things commented out in build system:
    • Lib::Graph
    • MatchTag
    • OptionsReader
    • FormulaIndex
    • CTFwSubsAndRes
    • CParser
    • EqualityAxiomatiser
    • GlobalOptions
    • Refutation
    • PDUtils
  • other fluff in build systems that doesn't exist any more
  • code guarded by #if GNUMP
  • methods in SAT::Preprocess
  • Inferences::SLQueryForwardSubsumption, Inferences::RefutationSeekerFSE
  • references to EGSubstitution
  • commented-out functions in Kernel/RobSubstitution
  • commented-out functions in Kernel/SortHelper
  • Kernel/Currifier.hpp
  • Term::askDistinctVars
  • "grounding" mode
  • Lib::LastCopyWatcher
  • Inferences::HyperSuperposition
  • PortfolioMode::waitForChildAndCheckIfProofFound
  • commented-out functions in Formula.cpp
  • DECLARE_PLACEMENT_NEW
  • TermSharing::insertRecurrently
  • Lib::TimeCounter
  • Shell::Profile
  • FoolAwareSubformulaIterator and FoolAwareSubtermIterator
  • InstGen
  • ArrayTheoryISE
  • Api/
  • LTB
  • env.out()
  • Lib::MemoryLeak
  • Various things in Matcher: OCMatchIterator, MatchIterator...
  • Minisat's limitMemory, we control its allocations.
  • Lib::MultiCounter
  • applyToLiteral in SubstHelper
  • Some of the scripts in scripts seem like they might no longer serve a purpose.
  • SortHelper::getTermSort
  • old question-answering logic, see Synthesis of non-recursive programs #442
  • combinatory HOL approach, superseded by HOL branch

Some things might be used in the near future, but currently are not.

  • CodeTreeSubsumptionIndex and ClauseCodeTree, orphaned by CTFwSubsAndRes - see below.

Credit to @quickbeam123, @ibnyusuf and @selig for identifying some of these. What I'd like from the team is help to identify more unused files or dead code, as well as rescuing files that need to be kept.

Eventually things that are still on the list will be removed by PR. Fear not, however, one of the many virtues of source control is the ability to reanimate code from beyond the grave. Thank you!

@quickbeam123
Copy link
Collaborator

What about the whole API folder?

There is also some mysterious looking ProofSimplifier in Shell.

@MichaelRawson
Copy link
Contributor Author

Very possibly @quickbeam123! I'm going to chat with Giles Monday about any politics I need to be aware of, then start deleting things in order of least-likely-to-be-wanted first.

@ibnyusuf
Copy link
Member

ibnyusuf commented Nov 8, 2020

Inferences/InterpretedSimplifier is completed commented out. Perhaps it can be removed.

@quickbeam123
Copy link
Collaborator

Some values in TimeCounterUnit most likely don't occur in the code anymore... Like TC_BDD_CLAUSIFICATION, etc. Could be removed as well!

This was referenced Nov 16, 2020
@quickbeam123
Copy link
Collaborator

There also some code, authored by Ioan, generally guarded by #if GNUMP and thus not compiled currently, which could perhaps be removed.

@quickbeam123
Copy link
Collaborator

Got another one: CTFwSubsAndRes.*

  • maybe useful if at some point want to revive code trees for subsumption, but currently not even compiled
  • by the way IndexManager holds a reference to CodeTreeSubsumptionIndex via FW_SUBSUMPTION_CODE_TREE, but only CTFwSubsAndRes would ever request this index.

So, transitively, CodeTreeSubsumptionIndex is also unused and maybe some other indices are formally alive but unused because of how IndexManager works.

@MichaelRawson
Copy link
Contributor Author

@quickbeam123 - good spot, I think @JakobR plans to fix something in this region - perhaps they can comment to see what they need?

@JakobR
Copy link
Member

JakobR commented Dec 9, 2020

@MichaelRawson feel free to remove CTFwSubsAndRes; I will revive it from the history in case I need it later.

It would be nice if we could keep CodeTreeSubsumptionIndex and ClauseCodeTree though (they're both compiled now, but probably unused). But if they're too much of a hassle to keep around, you can remove them as well and I will deal with it later.

@MichaelRawson
Copy link
Contributor Author

@JakobR - thanks. OK, CTFw can go but I'll keep code trees - I'm not in a hurry to delete stuff people want, only the stuff nobody wants. :-)

@MichaelRawson
Copy link
Contributor Author

@easychair and/or @quickbeam123 - a reminder, do you have some data about which options are/are not useful in terms of schedules? Naturally it's not the only consideration when removing something, but it's a pretty good indicator.

@quickbeam123
Copy link
Collaborator

I have lot's of data, but it's not so easy to interpret it. Most notably, it's not entirely clear where draw the line and call an option/value not worth maintaining. For most of the cases, one can find a problem that the database cannot solve otherwise... (then how many lost solution can be reclaimed by more search, or how many lost solutions are worth sacrificing for better maintenance?)

@MichaelRawson
Copy link
Contributor Author

Most notably, it's not entirely clear where draw the line and call an option/value not worth maintaining

I think this is the crux of it. Maybe we can start by ranking options in terms of effectiveness? Is that a reasonable thing to do?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants