all abstraction tests working again

Former-commit-id: 9de7beb2b6

src/abstraction/AbstractionInformation.cpp

@@ -9,7 +9,7 @@ namespace storm {
     namespace abstraction {
 
         template<storm::dd::DdType DdType>
-        AbstractionInformation<DdType>::AbstractionInformation(storm::expressions::ExpressionManager& expressionManager, std::shared_ptr<storm::dd::DdManager<DdType>> ddManager) : expressionManager(expressionManager), ddManager(ddManager), allPredicateIdentities(ddManager->getBddZero()) {
+        AbstractionInformation<DdType>::AbstractionInformation(storm::expressions::ExpressionManager& expressionManager, std::shared_ptr<storm::dd::DdManager<DdType>> ddManager) : expressionManager(expressionManager), ddManager(ddManager), allPredicateIdentities(ddManager->getBddOne()) {
             // Intentionally left empty.
         }
         
@@ -133,21 +133,21 @@ namespace storm {
             STORM_LOG_THROW(player1Variables.empty() && player2Variables.empty() && probabilisticBranchingVariables.empty(), storm::exceptions::InvalidOperationException, "Variables have already been created.");
             
             for (uint64_t index = 0; index < player1VariableCount; ++index) {
-                storm::expressions::Variable newVariable = ddManager->addMetaVariable("pl1" + std::to_string(index)).first;
+                storm::expressions::Variable newVariable = ddManager->addMetaVariable("pl1_" + std::to_string(index)).first;
                 player1Variables.push_back(newVariable);
                 player1VariableBdds.push_back(ddManager->getEncoding(newVariable, 1));
             }
             STORM_LOG_DEBUG("Created " << player1VariableCount << " player 1 variables.");
 
             for (uint64_t index = 0; index < player2VariableCount; ++index) {
-                storm::expressions::Variable newVariable = ddManager->addMetaVariable("pl2" + std::to_string(index)).first;
+                storm::expressions::Variable newVariable = ddManager->addMetaVariable("pl2_" + std::to_string(index)).first;
                 player2Variables.push_back(newVariable);
                 player2VariableBdds.push_back(ddManager->getEncoding(newVariable, 1));
             }
             STORM_LOG_DEBUG("Created " << player2VariableCount << " player 2 variables.");
 
             for (uint64_t index = 0; index < probabilisticBranchingVariableCount; ++index) {
-                storm::expressions::Variable newVariable = ddManager->addMetaVariable("pb" + std::to_string(index)).first;
+                storm::expressions::Variable newVariable = ddManager->addMetaVariable("pb_" + std::to_string(index)).first;
                 probabilisticBranchingVariables.push_back(newVariable);
                 probabilisticBranchingVariableBdds.push_back(ddManager->getEncoding(newVariable, 1));
             }
@@ -172,7 +172,7 @@ namespace storm {
         template<storm::dd::DdType DdType>
         storm::dd::Bdd<DdType> AbstractionInformation<DdType>::getPlayer2ZeroCube(uint_fast64_t numberOfVariables, uint_fast64_t offset) const {
             storm::dd::Bdd<DdType> result = ddManager->getBddOne();
-            for (uint_fast64_t index = offset; index < numberOfVariables - offset; ++index) {
+            for (uint_fast64_t index = offset; index < numberOfVariables; ++index) {
                 result &= player2VariableBdds[index];
             }
             STORM_LOG_ASSERT(!result.isZero(), "Zero cube must not be zero.");
@@ -184,16 +184,31 @@ namespace storm {
             return player1Variables;
         }
         
+        template<storm::dd::DdType DdType>
+        std::set<storm::expressions::Variable> AbstractionInformation<DdType>::getPlayer1VariableSet(uint_fast64_t count) const {
+            return std::set<storm::expressions::Variable>(player1Variables.begin(), player1Variables.begin() + count);
+        }
+        
         template<storm::dd::DdType DdType>
         std::vector<storm::expressions::Variable> const& AbstractionInformation<DdType>::getPlayer2Variables() const {
             return player2Variables;
         }
         
+        template<storm::dd::DdType DdType>
+        std::set<storm::expressions::Variable> AbstractionInformation<DdType>::getPlayer2VariableSet(uint_fast64_t count) const {
+            return std::set<storm::expressions::Variable>(player2Variables.begin(), player2Variables.begin() + count);
+        }
+        
         template<storm::dd::DdType DdType>
         std::vector<storm::expressions::Variable> const& AbstractionInformation<DdType>::getProbabilisticBranchingVariables() const {
             return probabilisticBranchingVariables;
         }
         
+        template<storm::dd::DdType DdType>
+        std::set<storm::expressions::Variable> AbstractionInformation<DdType>::getProbabilisticBranchingVariableSet(uint_fast64_t count) const {
+            return std::set<storm::expressions::Variable>(probabilisticBranchingVariables.begin(), probabilisticBranchingVariables.begin() + count);
+        }
+        
         template<storm::dd::DdType DdType>
         std::set<storm::expressions::Variable> const& AbstractionInformation<DdType>::getSourceVariables() const {
             return sourceVariables;

src/abstraction/AbstractionInformation.h

@@ -214,6 +214,14 @@ namespace storm {
              */
             std::vector<storm::expressions::Variable> const& getPlayer1Variables() const;
             
+            /*!
+             * Retrieves the set of player 1 variables.
+             *
+             * @param count The number of player 1 variables to include.
+             * @return The set of player 1 variables.
+             */
+            std::set<storm::expressions::Variable> getPlayer1VariableSet(uint_fast64_t count) const;
+            
             /*!
              * Retrieves the number of player 1 variables.
              *
@@ -235,6 +243,14 @@ namespace storm {
              */
             std::size_t getPlayer2VariableCount() const;
 
+            /*!
+             * Retrieves the set of player 2 variables.
+             *
+             * @param count The number of player 2 variables to include.
+             * @return The set of player 2 variables.
+             */
+            std::set<storm::expressions::Variable> getPlayer2VariableSet(uint_fast64_t count) const;
+
             /*!
              * Retrieves the meta variables associated with the probabilistic branching.
              *
@@ -242,6 +258,14 @@ namespace storm {
              */
             std::vector<storm::expressions::Variable> const& getProbabilisticBranchingVariables() const;
 
+            /*!
+             * Retrieves the set of probabilistic branching variables.
+             *
+             * @param count The number of probabilistic branching variables to include.
+             * @return The set of probabilistic branching variables.
+             */
+            std::set<storm::expressions::Variable> getProbabilisticBranchingVariableSet(uint_fast64_t count) const;
+
             /*!
              * Retrieves the number of probabilistic branching variables.
              *

src/abstraction/StateSetAbstractor.cpp

@@ -13,11 +13,6 @@ namespace storm {
         template <storm::dd::DdType DdType, typename ValueType>
         StateSetAbstractor<DdType, ValueType>::StateSetAbstractor(AbstractionInformation<DdType>& abstractionInformation, std::set<storm::expressions::Variable> const& allVariables, std::vector<storm::expressions::Expression> const& statePredicates, storm::utility::solver::SmtSolverFactory const& smtSolverFactory) : smtSolver(smtSolverFactory.create(abstractionInformation.getExpressionManager())), abstractionInformation(abstractionInformation), localExpressionInformation(allVariables), relevantPredicatesAndVariables(), concretePredicateVariables(), needsRecomputation(false), cachedBdd(abstractionInformation.getDdManager().getBddZero()), constraint(abstractionInformation.getDdManager().getBddOne()) {
             
-            // Assert all constraints to enforce legal variable values.
-            for (auto const& constraint : abstractionInformation.getConstraints()) {
-                smtSolver->add(constraint);
-            }
-            
             // Assert all state predicates.
             for (auto const& predicate : statePredicates) {
                 smtSolver->add(predicate);
@@ -27,13 +22,6 @@ namespace storm {
                 concretePredicateVariables.insert(usedVariables.begin(), usedVariables.end());
                 localExpressionInformation.relate(usedVariables);
             }
-            
-            // Refine the command based on all initial predicates.
-            std::vector<uint_fast64_t> allPredicateIndices(abstractionInformation.getNumberOfPredicates());
-            for (auto index = 0; index < abstractionInformation.getNumberOfPredicates(); ++index) {
-                allPredicateIndices[index] = index;
-            }
-            this->refine(allPredicateIndices);
         }
         
         template <storm::dd::DdType DdType, typename ValueType>
@@ -58,6 +46,11 @@ namespace storm {
             needsRecomputation = true;
         }
         
+        template <storm::dd::DdType DdType, typename ValueType>
+        void StateSetAbstractor<DdType, ValueType>::constrain(storm::expressions::Expression const& constraint) {
+            smtSolver->add(constraint);
+        }
+        
         template <storm::dd::DdType DdType, typename ValueType>
         void StateSetAbstractor<DdType, ValueType>::constrain(storm::dd::Bdd<DdType> const& newConstraint) {
             // If the constraint is different from the last one, we add it to the solver.
@@ -109,7 +102,7 @@ namespace storm {
             
             storm::dd::Bdd<DdType> result = this->getAbstractionInformation().getDdManager().getBddZero();
             uint_fast64_t modelCounter = 0;
-            smtSolver->allSat(decisionVariables, [&result,this,&modelCounter] (storm::solver::SmtSolver::ModelReference const& model) { result |= getStateBdd(model); return true; } );
+            smtSolver->allSat(decisionVariables, [&result,this,&modelCounter] (storm::solver::SmtSolver::ModelReference const& model) { result |= getStateBdd(model); ++modelCounter; return true; } );
             
             cachedBdd = result;
         }

src/abstraction/StateSetAbstractor.h

@@ -60,6 +60,14 @@ namespace storm {
              */
             void refine(std::vector<uint_fast64_t> const& newPredicateIndices);
             
+            /*!
+             * Constrains the abstract states with the given expression.
+             * Note that all constaints must be added before the abstractor is used to retrieve the abstract states.
+             *
+             * @param constraint The constraint to add.
+             */
+            void constrain(storm::expressions::Expression const& constraint);
+            
             /*!
              * Constraints the abstract states with the given BDD.
              *

src/abstraction/prism/AbstractCommand.cpp

@@ -297,7 +297,7 @@ namespace storm {
                 for (uint_fast64_t updateIndex = 0; updateIndex < command.get().getNumberOfUpdates(); ++updateIndex) {
                     result += this->getAbstractionInformation().encodeProbabilisticChoice(updateIndex, this->getAbstractionInformation().getProbabilisticBranchingVariableCount()).template toAdd<ValueType>() * this->getAbstractionInformation().getDdManager().getConstant(evaluator.asRational(command.get().getUpdate(updateIndex).getLikelihoodExpression()));
                 }
-                result *= this->getAbstractionInformation().encodePlayer2Choice(command.get().getGlobalIndex(), this->getAbstractionInformation().getPlayer2VariableCount()).template toAdd<ValueType>();
+                result *= this->getAbstractionInformation().encodePlayer1Choice(command.get().getGlobalIndex(), this->getAbstractionInformation().getPlayer1VariableCount()).template toAdd<ValueType>();
                 return result;
             }
             

src/abstraction/prism/AbstractModule.cpp

@@ -27,11 +27,6 @@ namespace storm {
                 }
             }
             
-            template <storm::dd::DdType DdType, typename ValueType>
-            AbstractModule<DdType, ValueType>::~AbstractModule() {
-                std::cout << "destructing abs module at " << this << std::endl;
-            }
-            
             template <storm::dd::DdType DdType, typename ValueType>
             std::pair<storm::dd::Bdd<DdType>, uint_fast64_t> AbstractModule<DdType, ValueType>::getAbstractBdd() {
                 // First, we retrieve the abstractions of all commands.

src/abstraction/prism/AbstractModule.h

@@ -31,13 +31,11 @@ namespace storm {
                  */
                 AbstractModule(storm::prism::Module const& module, AbstractionInformation<DdType>& abstractionInformation, storm::utility::solver::SmtSolverFactory const& smtSolverFactory);
                 
-                AbstractModule(AbstractModule const&) = delete;
-                AbstractModule& operator=(AbstractModule const&) = delete;
+                AbstractModule(AbstractModule const&) = default;
+                AbstractModule& operator=(AbstractModule const&) = default;
                 AbstractModule(AbstractModule&&) = default;
                 AbstractModule& operator=(AbstractModule&&) = default;
                 
-                ~AbstractModule();
-                
                 /*!
                  * Refines the abstract module with the given predicates.
                  *

src/abstraction/prism/AbstractProgram.cpp

@@ -16,17 +16,12 @@ namespace storm {
     namespace abstraction {
         namespace prism {
             
-            template <storm::dd::DdType DdType, typename ValueType>
-            AbstractProgram<DdType, ValueType>::~AbstractProgram() {
-                std::cout << "destructing abs program" << std::endl;
-            }
-            
             template <storm::dd::DdType DdType, typename ValueType>
             AbstractProgram<DdType, ValueType>::AbstractProgram(storm::expressions::ExpressionManager& expressionManager, storm::prism::Program const& program,
                                                                 std::vector<storm::expressions::Expression> const& initialPredicates,
                                                                 std::unique_ptr<storm::utility::solver::SmtSolverFactory>&& smtSolverFactory,
                                                                 bool addAllGuards)
-            : program(program), smtSolverFactory(std::move(smtSolverFactory)), modules(), abstractionInformation(expressionManager), initialStateAbstractor(abstractionInformation, program.getAllExpressionVariables(), {program.getInitialConstruct().getInitialStatesExpression()}, *this->smtSolverFactory), addedAllGuards(addAllGuards), bottomStateAbstractor(abstractionInformation, program.getAllExpressionVariables(), program.getAllGuards(true), *this->smtSolverFactory), currentGame(nullptr) {
+            : program(program), smtSolverFactory(std::move(smtSolverFactory)), abstractionInformation(expressionManager), modules(), initialStateAbstractor(abstractionInformation, program.getAllExpressionVariables(), {program.getInitialConstruct().getInitialStatesExpression()}, *this->smtSolverFactory), addedAllGuards(addAllGuards), bottomStateAbstractor(abstractionInformation, program.getAllExpressionVariables(), program.getAllGuards(true), *this->smtSolverFactory), currentGame(nullptr) {
                 
                 // For now, we assume that there is a single module. If the program has more than one module, it needs
                 // to be flattened before the procedure.
@@ -38,6 +33,8 @@ namespace storm {
                 }
                 for (auto const& range : this->program.get().getAllRangeExpressions()) {
                     abstractionInformation.addConstraint(range);
+                    initialStateAbstractor.constrain(range);
+                    bottomStateAbstractor.constrain(range);
                 }
                 
                 uint_fast64_t totalNumberOfCommands = 0;
@@ -61,13 +58,14 @@ namespace storm {
                 abstractionInformation.createEncodingVariables(static_cast<uint_fast64_t>(std::ceil(std::log2(totalNumberOfCommands))), 100, static_cast<uint_fast64_t>(std::ceil(std::log2(maximalUpdateCount))));
                 
                 // Now that we have created all other DD variables, we create the DD variables for the predicates.
+                std::vector<uint_fast64_t> allPredicateIndices;
                 if (addAllGuards) {
                     for (auto const& guard : allGuards) {
-                        abstractionInformation.addPredicate(guard);
+                        allPredicateIndices.push_back(abstractionInformation.addPredicate(guard));
                     }
                 }
                 for (auto const& predicate : initialPredicates) {
-                    abstractionInformation.addPredicate(predicate);
+                    allPredicateIndices.push_back(abstractionInformation.addPredicate(predicate));
                 }
                 
                 // For each module of the concrete program, we create an abstract counterpart.
@@ -75,13 +73,15 @@ namespace storm {
                     this->modules.emplace_back(module, abstractionInformation, *this->smtSolverFactory);
                 }
                 
+                // Refine the state abstractors using the initial predicates.
+                initialStateAbstractor.refine(allPredicateIndices);
+                bottomStateAbstractor.refine(allPredicateIndices);
+                
                 // Retrieve the command-update probability ADD, so we can multiply it with the abstraction BDD later.
                 commandUpdateProbabilitiesAdd = modules.front().getCommandUpdateProbabilitiesAdd();
                 
                 // Finally, we build the game the first time.
                 currentGame = buildGame();
-                
-                std::cout << "abs module at " << &modules.front() << std::endl;
             }
             
             template <storm::dd::DdType DdType, typename ValueType>
@@ -129,15 +129,17 @@ namespace storm {
                 std::pair<storm::dd::Bdd<DdType>, uint_fast64_t> gameBdd = modules.front().getAbstractBdd();
 
                 // Construct a set of all unnecessary variables, so we can abstract from it.
-                std::set<storm::expressions::Variable> variablesToAbstract(abstractionInformation.getPlayer1Variables().begin(), abstractionInformation.getPlayer1Variables().end());
-                variablesToAbstract.insert(abstractionInformation.getPlayer2Variables().begin(), abstractionInformation.getPlayer2Variables().end());
-                variablesToAbstract.insert(abstractionInformation.getProbabilisticBranchingVariables().begin(), abstractionInformation.getProbabilisticBranchingVariables().begin() + gameBdd.second);
-
+                std::set<storm::expressions::Variable> variablesToAbstract(abstractionInformation.getPlayer1VariableSet(abstractionInformation.getPlayer1VariableCount()));
+                auto player2Variables = abstractionInformation.getPlayer2VariableSet(gameBdd.second);
+                variablesToAbstract.insert(player2Variables.begin(), player2Variables.end());
+                auto probBranchingVariables = abstractionInformation.getProbabilisticBranchingVariableSet(abstractionInformation.getProbabilisticBranchingVariableCount());
+                variablesToAbstract.insert(probBranchingVariables.begin(), probBranchingVariables.end());
+                
                 // Do a reachability analysis on the raw transition relation.
                 storm::dd::Bdd<DdType> transitionRelation = gameBdd.first.existsAbstract(variablesToAbstract);
                 storm::dd::Bdd<DdType> initialStates = initialStateAbstractor.getAbstractStates();
                 storm::dd::Bdd<DdType> reachableStates = this->getReachableStates(initialStates, transitionRelation);
-
+                
                 // Determine the bottom states.
                 storm::dd::Bdd<DdType> bottomStates;
                 if (addedAllGuards) {

src/abstraction/prism/AbstractProgram.h

@@ -49,7 +49,6 @@ namespace storm {
                 AbstractProgram& operator=(AbstractProgram const&) = default;
                 AbstractProgram(AbstractProgram&&) = default;
                 AbstractProgram& operator=(AbstractProgram&&) = default;
-                ~AbstractProgram();
                 
                 /*!
                  * Uses the current set of predicates to derive the abstract menu game in the form of an ADD.
@@ -98,12 +97,12 @@ namespace storm {
                 // A factory that can be used to create new SMT solvers.
                 std::unique_ptr<storm::utility::solver::SmtSolverFactory> smtSolverFactory;
                 
+                // An object containing all information about the abstraction like predicates and the corresponding DDs.
+                AbstractionInformation<DdType> abstractionInformation;
+                
                 // The abstract modules of the abstract program.
                 std::vector<AbstractModule<DdType, ValueType>> modules;
                 
-                // An object containing all information about the abstraction like predicates and the corresponding DDs.
-                AbstractionInformation<DdType> abstractionInformation;
-
                 // A state-set abstractor used to determine the initial states of the abstraction.
                 StateSetAbstractor<DdType, ValueType> initialStateAbstractor;