Factor out countTrainableParams()

[davidsoergel]

Factor out and expose layer_utils/countTrainableParams(), because that's useful.

---

This change is 

[caisq]

Reviewable status: 0 of 1 approvals obtained (waiting on @davidsoergel and @caisq)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

export 

If this function is not used in any other files, remove this export.

[davidsoergel]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @davidsoergel)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

Previously, caisq (Shanqing Cai) wrote…

export 

If this function is not used in any other files, remove this export.

Ah but I want to use it from another repo, that's the whole idea :) We don't need to expose it at the index.ts level-- with this export I can still do this:

import { countParamsInWeights } from '@tensorflow/tfjs-layers/dist/utils/layer_utils';

[bileschi]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @caisq)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

Previously, davidsoergel (David Soergel) wrote…

Ah but I want to use it from another repo, that's the whole idea :) We don't need to expose it at the index.ts level-- with this export I can still do this:

import { countParamsInWeights } from '@tensorflow/tfjs-layers/dist/utils/layer_utils';

Curious about this pattern. Do we have other functions that are importable directly, but not exposed via our public api as set by the index? What sorts of guarantees / promises do we make about those?

[nsthorat]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @caisq)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

Previously, bileschi (Stanley Bileschi) wrote…

Curious about this pattern. Do we have other functions that are importable directly, but not exposed via our public api as set by the index? What sorts of guarantees / promises do we make about those?

You definitely should not import like that. We've had several breakages from file-level imports. If you want this externally, export it from the public API.

[caisq]

Reviewable status: complete! 1 of 1 approvals obtained (waiting on @davidsoergel and @bileschi)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

Previously, nsthorat (Nikhil Thorat) wrote…

You definitely should not import like that. We've had several breakages from file-level imports. If you want this externally, export it from the public API.

+1 to @nsthorat. Also, since this function is quite small, you can just duplicate the code in the client repo (the refactoring is good though).

[davidsoergel]

Reviewable status: 0 of 1 approvals obtained (waiting on @bileschi and @nsthorat)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

Previously, caisq (Shanqing Cai) wrote…

+1 to @nsthorat. Also, since this function is quite small, you can just duplicate the code in the client repo (the refactoring is good though).

OK, I removed the export. And yes I already have a pasted copy in the other repo where I need it :)

As for not using this pattern: OK, but then we should get rid of the other instances too. Converter, Layers, and Data all do this to get at core types, tensor_types, tensor_util, test_util, and jasmine_util-- so it sounds like we should discuss (not here) exposing those properly from core.

[nsthorat]

Correct, we should fix that everywhere. The one exception is jasmine_util, which depends on jasmine. If we exported this from index.ts, then we would bloat the standalone bundle with jasmine dependencies.

[caisq]

Reviewable status: 0 of 1 approvals obtained (waiting on @bileschi and @nsthorat)

---

src/utils/layer_utils.ts, line 89 at r1 (raw file):

Previously, davidsoergel (David Soergel) wrote…

OK, I removed the export. And yes I already have a pasted copy in the other repo where I need it :)

As for not using this pattern: OK, but then we should get rid of the other instances too. Converter, Layers, and Data all do this to get at core types, tensor_types, tensor_util, test_util, and jasmine_util-- so it sounds like we should discuss (not here) exposing those properly from core.

Agreed. I think many of those instances were results of inadvertently using VSCode's auto completion. We should go and fix those. I filed issue tensorflow/tfjs#575 to track this.