Multiobjective exportschedulers #328
Conversation
|
Thanks a lot for the contribution! I'll have a look at this once I find the time. |
…tates deleted in preprocessing
|
Hey! I've been improving the code and making it work for every scenario I need it, checked it out with some examples in the repo and I think is ok. But I couldn't write any tests because I don't find a way to set the "exportscheduler" setting within the unit test. I think that other than that is ready for review. |
| @@ -36,9 +43,19 @@ class ExplicitParetoCurveCheckResult : public ParetoCurveCheckResult<ValueType> | |||
|
|
|||
| storm::storage::sparse::state_type const& getState() const; | |||
|
|
|||
| virtual bool hasScheduler() const override; | |||
| // void addScheduler(const std::shared_ptr<storm::storage::Scheduler<ValueType>>& scheduler); | |||
| typename ParetoCurveCheckResult<ValueType>::polytope_type&& underApproximation, | ||
| typename ParetoCurveCheckResult<ValueType>::polytope_type&& overApproximation) | ||
| : ParetoCurveCheckResult<ValueType>(points, underApproximation, overApproximation), state(state), schedulers(schedulers) { | ||
| // Intentionally left empty. |
| template void transformObjectiveSchedulersToOriginal( | ||
| storm::storage::SparseModelMemoryProductReverseData const& reverseData, | ||
| std::shared_ptr<storm::models::sparse::MarkovAutomaton<storm::RationalNumber>> const& originalModel, | ||
| // std::shared_ptr<std::map<std::vector<storm::RationalNumber>, std::shared_ptr<storm::storage::Scheduler<storm::RationalNumber>>>> schedulers); |
|
|
||
| template void transformObjectiveSchedulersToOriginal( | ||
| storm::storage::SparseModelMemoryProductReverseData const& reverseData, std::shared_ptr<storm::models::sparse::Mdp<double>> const& originalModel, | ||
| // std::shared_ptr<std::map<std::vector<double>, std::shared_ptr<storm::storage::Scheduler<double>>>> schedulers); |
| template void transformObjectiveSchedulersToOriginal( | ||
| storm::storage::SparseModelMemoryProductReverseData const& reverseData, | ||
| std::shared_ptr<storm::models::sparse::Mdp<storm::RationalNumber>> const& originalModel, | ||
| // std::shared_ptr<std::map<std::vector<storm::RationalNumber>, std::shared_ptr<storm::storage::Scheduler<storm::RationalNumber>>>> schedulers); |
|
This looks nice! Thanks for your contribution. I made some minor remarks, I will leave the content review to TQ. Generally, the code in model-handling.h would profit from having a has_scheduler on the checkresult level, correct? |
|
Hi, I removed the leftover comments thanks for pointing that out. I guess if more checkresults types would implement scheduler export it would make sense to have hasScheduler in the checkresult level, but at some point casting the checkresult would still be needed for different cases, in this case looping over many schedulers. Or perhaps each subclass of checkresult could include also a export scheduler method and model-handling.h just calls it if hasScheduler is true. But I don't know if we should delegate stream handling responsibilities to checkresult subclasses or keep it within the strorm-cli module. |
|
I've been using this branch and I found a slight change in the MDP that brakes the code. I'll try to fix it when I have some time. |
|
@UlisesTorrella what is the status of the bug that you mentioned? |
|
Hey sorry, it's been a busy time. I'll try to take a look soon. |
Hello, this PR refers to this issue, I needed to export the schedulers for each pareto optimal point when performing multiobjective analysis on MDPs. I focused on my direct need, and it worked with [this commit] because it had no memory incorporation, I tried to make it work for the general case but not knowing much of the subject I'm unsure of my solution. Here's what i've done:
I included a point->scheduler mapping in
ExplicitParetoCurveCheckResultandSparsePcaaParetoQueryto build it as the points are found.I had to make to transformations to each scheduler:
StandardPcaaWeightVectorCheckerlevel to match the optimal choices from a scheduler for a model with merged goalds (hence themergerResultneeded to store in the vector checker).transformObjectiveSchedulersToOriginaliterates over the mapping matching the choices for the preprocessed model to the target model. For this I had to retrieve mapping data from the MemoryIncorporation step during preprocessing in order to reverse the procedure, and it's the part is what I'm highly unsure is correct. I obtain aMemoryStructureand atoResultStateMappingin order to convert a scheduler for a SparseModel with incorporated memory into a scheduler for a target model with memory.This PR is a draft, I'm in the process of building tests based on example models and not rely only on my MDPs. But I publish it so anyone can point out problems that might surge from my implementation that would require me to go back and re implement it.
As memory usage concerns, I guess the point->scheduler mapping is just necessary to the task, I tried to keep the data to reverse memory incorporation to a minimum creating the struct
SparseModelMemoryProductReverseData. And for the GoalStateMerger result I stored the ReturnType completly in the weight vector checker, perhaps it could be less.As for algorithmic mistakes, there are 2 places to look at,
computeOriginalSchedulerandtransformObjectiveSchedulersToOriginal, both are looping over states and memory to build a scheduler based on another.Also I changed the signature of
incorporateGoalMemory, I checked for uses inside the project, but I'm unaware of any other uses. If needed it could be wrapped in a new function.Let me know what needs to be changed, this is my first time using c++ so I'm sorry for any gross mistake.