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

Improve documentation and introduce simple synthetic examples #4

Merged
merged 35 commits into from
Dec 3, 2023

Conversation

slwu89
Copy link
Collaborator

@slwu89 slwu89 commented Nov 16, 2023

Hi @thevolatilebit this is a draft PR to address some of the documentation topics we discussed in the enterprise internal repo. As a reminder, what I would eventually like this PR to include is:

  • Move duplicated theoretical parts out of the existing tutorials into 2 separate tutorials based on introducing the theory/mathematical preliminaries behind static and generative designs.
  • Make simple examples for static and generative designs based on synthetic data that may be used to show just the theory and potentially in our tests to speed up tests, which currently take some time to run.

The synthetic example for static designs works great and is producing expected results with the input data.

That all being said, I am having trouble getting synthetic examples with known properties to work for the generative designs. In this file https://github.com/Merck/CEED.jl/blob/8786d768198bad2f1d5b77013019c93291760c05/tutorials/SimpleGenerative.jl I have attempted to set up an example with both a continuous and discrete outcome. However, for both cases, the MDP solution is always the empty set, despite the features contributing information to predicting the outcome, by construction. @thevolatilebit I am wondering if you can help me look at those 2 examples to figure out why this is the case, I am very curious why I cannot get different actions proposed.

@thevolatilebit
Copy link
Collaborator

Thanks, @slwu89, for the fantastic tutorials.

Two reasons explain why other designs were not displayed: Firstly, even after running experiments e1 to e4, the uncertainty still exceeds the threshold, resulting in a "bigM" penalty: https://github.com/Merck/CEED.jl/blob/b803dfddd818d3844f33b3967201dd1c7535e1ef/src/GenerativeDesigns/UncertaintyReductionMDP.jl#L153-L155

Secondly, a filter for "degenerate" designs with too high cost existed in: https://github.com/Merck/CEED.jl/blob/6d932f6a69bdb7906ae8c1dd82c9a1008c336010/src/fronts.jl#L108-L110

I have now removed this condition.

@thevolatilebit
Copy link
Collaborator

thevolatilebit commented Nov 23, 2023

It would be great to eventually feature the Mahalanobis distance (#7) in the synthetic examples as well.

@slwu89
Copy link
Collaborator Author

slwu89 commented Nov 30, 2023

@thevolatilebit, I've included Mahalanobis distance (#7) in the generative tutorial now. Still need to spend more time on the explanatory text before it's ready for your review.

@slwu89
Copy link
Collaborator Author

slwu89 commented Nov 30, 2023

@thevolatilebit can you help me take a look at tutorials/SimpleGenerative.jl and tutorials/SimpleStatic.jl and make sure that they are set up correctly with the rest of the Project.toml files and things, for building docs? When I run the docs/make.jl file I get errors of the sort pasted below from the new tutorial files. For example, Distributions is in tutorials/Project.toml but not in docs/Project.toml, and I am not sure if it is supposed to be there too. Thanks!

┌ Warning: failed to run `@example` block in src/tutorials/SimpleGenerative.md:148-156
│ ```@example SimpleGenerative
│ using CEEDesigns, CEEDesigns.GenerativeDesigns
│ using DataFrames
│ using ScientificTypes
│ import Distributions
│ using Copulas
│ using POMDPs, POMDPTools, MCTS
│ using Plots, StatsPlots, StatsBase
│ ```
│   value =
│    ArgumentError: Package Distributions not found in current path.
│    - Run `import Pkg; Pkg.add("Distributions")` to install the Distributions package.
└ @ Documenter.Expanders ~/.julia/packages/Documenter/bYYzK/src/Utilities/Utilities.jl:34

@thevolatilebit
Copy link
Collaborator

@thevolatilebit can you help me take a look at tutorials/SimpleGenerative.jl and tutorials/SimpleStatic.jl and make sure that they are set up correctly with the rest of the Project.toml files and things, for building docs? When I run the docs/make.jl file I get errors of the sort pasted below from the new tutorial files. For example, Distributions is in tutorials/Project.toml but not in docs/Project.toml, and I am not sure if it is supposed to be there too. Thanks!

┌ Warning: failed to run `@example` block in src/tutorials/SimpleGenerative.md:148-156
│ ```@example SimpleGenerative
│ using CEEDesigns, CEEDesigns.GenerativeDesigns
│ using DataFrames
│ using ScientificTypes
│ import Distributions
│ using Copulas
│ using POMDPs, POMDPTools, MCTS
│ using Plots, StatsPlots, StatsBase
│ ```
│   value =
│    ArgumentError: Package Distributions not found in current path.
│    - Run `import Pkg; Pkg.add("Distributions")` to install the Distributions package.
└ @ Documenter.Expanders ~/.julia/packages/Documenter/bYYzK/src/Utilities/Utilities.jl:34

@slwu89 Yeah, it is necessary to add all the dependencies to docs/Project.toml as well. Should be fixed now.

@slwu89
Copy link
Collaborator Author

slwu89 commented Dec 1, 2023

Thanks.

@slwu89 slwu89 marked this pull request as ready for review December 1, 2023 19:17
Optimizes code in `distancebased.jl`.

Changes the way that an "uncertainty" is provided. That is,
while "distances" and "similarities" are provided by a user through
 a function call, an "uncertainty" was originally given as a type.

Fixes #13
- Implemented a variant of sq. Mahalanobis distance
with missing entries, see https://www.jstor.org/stable/3559861
on page 285, fixes #12
- Renamed `MahalanobisDistance` to `SquaredMahalanobisDistance`
- Minor adjustments to tutorials
- Removed extra dependencies
- Incremented version

Fixes #11 and #12
Copy link
Collaborator

@thevolatilebit thevolatilebit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent job, thank you, @slwu89 !

@thevolatilebit thevolatilebit merged commit 87ac18c into main Dec 3, 2023
3 checks passed
@slwu89 slwu89 deleted the docs branch December 3, 2023 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants