tensorflow / tensorflow/java

API convention to take runtime type parameters in constructors of tensor-type-parameterized classes?

オープン
#201 コメント 11 件 リアクション 0 件 担当者 0 名 GitHub で見る

まだ誰も着手していません。

主要言語
Java
スター
928
フォーク
227
PR マージ指標
30日以内にマージされた PR はありません

説明

Our API design deliberately uses lots of tensor-type-parameterized classes. Here's a typical example:

public class BinaryCrossentropy<U extends TNumber, T extends TNumber>
    extends MeanMetricWrapper<U, T> implements LossMetric<T> {

Of course, Java generic type parameters are erased at runtime, so the implementation of this class doesn't necessarily have Class objects for types U and T if it needs them.

In a few cases, our initial implementation of such classes has already found itself in this situation, so we have required Class parameters in constructors. Here's an example:

public class MeanMetricWrapper<U extends TNumber, T extends TNumber> extends Mean<U, T> {
   . . .
  protected MeanMetricWrapper(Ops tf, String name, long seed, Class<T> type) {
    super(tf, name, seed, type);
  }

But suppose we didn't need a stored runtime class when we originally implemented this class, but then eventually made changes that required it? (Or suppose we need the runtime type for U!) To evolve the constructor API, we would have to choose between unpleasant alternatives:

  • Add a new constructor that takes the runtime class, and make it a runtime error to use the new functionality if you didn't use that constructor.
  • Punt on adding the new functionality to the existing class hierarchy, perhaps instead creating a MeanMetricWrapperV2 and then also forking subclasses as needed.
  • Or make a breaking change by adding a runtime type parameter to existing constructors (which in this case would break a ton of subclass APIs).

I wonder whether we should make it a consistent pattern that classes parameterized by tensor types take the runtime types in their constructors?

In Java, this will make constructor invocation less convenient. Here's an example in existing code:

      BinaryCrossentropy<TFloat32, TFloat64> instance =
          new BinaryCrossentropy<>(tf, "BCE_testUnweighted", false, 0, 1001L, TFloat64.class);

Note the TFloat64.class at the end of the call. In Java 1.8, this is entirely an added burden: it is redundant with the second TFloat64 type parameter, but both are required. However, with the addition of local variable type inference in Java 10, this redundancy will be eliminated. The only extra burden then will be the .class. In Kotlin, this could be entirely smoothed over with inline factory methods.

コントリビューションガイド

コントリビューションガイドを開く

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

調査の方向性

まず issue にある BinaryCrossentropy と MeanMetricWrapper のコンストラクター例を読み、次に周辺のテンソル型パラメーター化されたクラス階層を調べます。実行時型パラメーターを一貫して必須にすべきか、またそれが既存のサブクラス API にどのような影響を与えるかを判断します。API 規約と移行方針について合意できれば完了です。

索引モデルが issue の本文から書いたものです。

評価

技術スタック
java
領域
api, backend-api-design
issue の種類
機能追加
難易度
5/5
見積もり時間
1週間以上
活発さ
停滞
明瞭さ
説明が足りない
初心者へのやさしさ
20/100

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。