masa-su / pixyz

A library for developing deep generative models in a more concise, intuitive and extendable way
https://pixyz.io
MIT License
487 stars 41 forks source link

fix/Unupdated graph name of an atomic distribution #145

Closed ktaaaki closed 3 years ago

ktaaaki commented 3 years ago

※先にfeature/add_pytest_v3をマージしてください.

AtomicなDistributionのnameプロパティを変更しても,AtomicなDistributionを使用するグラフで変更が反映されない問題. → 以下の「反映されるべき」を採用

原因と対策

反映されないべき

Distributionは式木でありconst扱いするべきで,設定可能なプロパティが存在するのが間違っている

→ name.setterはMultiplyDistributionなどのoperatorを使った生成でinitを通じた初期化ができないときに使用されているため,AtomicDistribution以外から初回のみ設定可能にすれば分かりやすいはず  → 初期化フラグを状態として追加したくない(わかりにくい)

反映されるべき

DistGraphをDistribution APIに封じ込めるためのlife spanの設計に問題がある

この設計は,DistGraphのstrでAtomicDistribution.reprをnn.Moduleとして呼び出すために,DistGraphをcompositionすると無限ループするから(つまりnn.Moduleメタ機構の問題)

→ nn.Moduleのmodulesに検出されないようにcompositionする