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

Add start_sigma to ADVI #6096

Merged
merged 3 commits into from
Sep 14, 2022
Merged

Conversation

markusschmaus
Copy link
Contributor

@markusschmaus markusschmaus commented Sep 2, 2022

Introduces the start_sigma argument which allows to set a starting value for the sigmas of mean field approximation in ADVI. I am using the ordering property to create the mapping between the variables and the flat 1d array required by ADVI.

See also:
https://discourse.pymc.io/t/quality-of-life-improvements-to-advi/10254

Checklist

Major / Breaking Changes

  • None

Bugfixes / New features

  • Allow start_sigma being given to fit.
  • For ASVGD forward the key word arguments start and random_seed to the constructor of the default approximation FullRank instead of sending them to super().__init__ which can't process them
  • Enable _iterate_with_loss to run with n=0 by giving an appropriate logger message. This allows to check initialization values, which is useful for testing, but could also be useful for debugging a user model

Docs / Maintenance

  • The documentation for the start argument was outdated, it claimed type Point, but it gets passed through to a function that requires StartDict. Since users are probably unfamiliar with this custom type (Dict[Union[Variable, str], Union[np.ndarray, Variable, str]]), I decided to also mention the most relevant and most commonly used subtype dict[str, np.ndarray].

@markusschmaus markusschmaus marked this pull request as ready for review September 3, 2022 11:14
@junpenglao junpenglao requested a review from ferrine September 6, 2022 21:04
@codecov
Copy link

codecov bot commented Sep 6, 2022

Codecov Report

Merging #6096 (9a894ce) into main (c858f0f) will increase coverage by 3.10%.
The diff coverage is 47.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #6096      +/-   ##
==========================================
+ Coverage   87.29%   90.39%   +3.10%     
==========================================
  Files          72       86      +14     
  Lines       12927    17873    +4946     
==========================================
+ Hits        11284    16157    +4873     
- Misses       1643     1716      +73     
Impacted Files Coverage Δ
pymc/variational/inference.py 50.77% <37.50%> (-0.56%) ⬇️
pymc/variational/approximations.py 54.79% <55.55%> (-0.61%) ⬇️
pymc/distributions/shape_utils.py 98.43% <0.00%> (-0.84%) ⬇️
pymc/printing.py 86.08% <0.00%> (-0.58%) ⬇️
pymc/distributions/censored.py 92.50% <0.00%> (-0.53%) ⬇️
pymc/distributions/logprob.py 97.31% <0.00%> (-0.42%) ⬇️
pymc/distributions/mixture.py 95.40% <0.00%> (-0.32%) ⬇️
pymc/sampling.py 82.13% <0.00%> (-0.02%) ⬇️
pymc/distributions/__init__.py 100.00% <0.00%> (ø)
pymc/distributions/continuous.py 97.51% <0.00%> (ø)
... and 20 more

@junpenglao junpenglao merged commit ec27b5c into pymc-devs:main Sep 14, 2022
ricardoV94 added a commit to ricardoV94/pymc that referenced this pull request Sep 15, 2022
@ricardoV94 ricardoV94 added the no releasenotes Skipped in automatic release notes generation label Sep 15, 2022
junpenglao pushed a commit that referenced this pull request Sep 15, 2022
@junpenglao
Copy link
Member

Hi @markusschmaus sorry we need to revert this because there is a test failing - could you send in the PR again?

@markusschmaus markusschmaus mentioned this pull request Sep 15, 2022
5 tasks
@markusschmaus
Copy link
Contributor Author

markusschmaus commented Sep 15, 2022

@junpenglao
Sorry for missing the failing test. This was caused by me fixing a "bug" which turned out not to be a bug after all.

Here is the new PR:
#6132

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no releasenotes Skipped in automatic release notes generation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants