Couple questions RE tensor stuff

[danpovey]

I have a couple of questions RE the Tensor stuff I'm working on for kaldi10.

It wasn't really possible to avoid the use of shared_ptr (although if we don't need
multi-threaded operation, we could of course always replace them later with something
that doesn't use atomics).

When I write code involving passing shared_ptr around, sometimes I do

    void Foo (const std::shared_ptr<Bar> &bar);

with the logic that passing a const reference to the shared_ptr should be much faster than
copying the shared_ptr (which would involve changing its ref-count, hence atomics).
Is this reasonable? @kkm000? It just looks odd to me.

Secondly, I want to augment Variable with a mechanism to (optionally) store
arbitrary configuration information. It's in case we need any of those types of
mechanisms we use in Kaldi, where, for instance, we set different amounts of
l2 on different parameters, or specify an orthonormal constraint for certain
parameters, etc. (Parameter groups, like PyTorch uses, are
super-inconvenient and also don't really scale beyond a single configurable
parameter). One possibility is a map from string to string, but it seems wrong to
have to parse strings anytime you need a configuration variable. Another
possibility is to map from string to some kind of struct that can store a bool,
an int or a string (for instance; could maybe be augmented later). Of course
we'd have to figure out how to reflect this to Python, but it should be fairly easy
since it's pretty much in the spirit of how Python works. I just wondered whether
anyone has any comments, and whether there is a standard way to do this?

[kkm000]

It wasn't really possible to avoid the use of shared_ptr

The main reason the warning about using shared_ptr judiciously is usually given is because it is the direct replacement of the STL ill-concieved auto_ptr. There is absolutely nothing wrong about using shared pointers where reference counting is needed.

replace them later with something that doesn't use atomics.

Atomics generally should not be expensive. AFAIK, atomic loads and saves cost nothing on Intel CPUs. Atomic increment/decrements cost more in the case of contention, but I do not think that absent contention they would add anything at all to the cost of operations on things we are usually pointing to with smart pointers, like matrix slices. I would not worry about atomic vs non atomic even for single thread access (and it's better to be correct than fast, anyway).

void Foo (const std::shared_ptr &bar);

So the idea here is that Foo() decides whether or not it wants to keep a copy of the shared pointer? (Otherwise a const Bar* would do). I see nothing wrong with that.

augment Variable with a mechanism to (optionally) store arbitrary configuration information [...] whether there is a standard way to do this?

C++17 has std::any. Maybe it's time to bite the bullet and bump the required C++ language version; all major compilers have supported it for quite a while. As a second option, there is also boost::any, which comes pretty much in a single header under a permissive license, so it can be just checked in.

[kkm000]

Regarding the shared_ptr by const reference, there is a good answer: https://stackoverflow.com/a/8385581/1149924

Another one summarizes Meyers and Sutter (and who comes by first name Anderi?) response as “Shortly, there is no reason to pass [shared_ptr] by value, unless the goal is to share ownership of an object.”

So it's a totally valid thing to do.

[kkm000]

There is also std::variant, also C++17. This thing allows only one of the specified types, like std::variant<string, int, float>.

[danpovey]

OK. I think I'll roll my own. Obviously would be more elegant to use the C++17 thingy, but if it makes compilation easier on at least one system, it's worth it to stay with C++11 IMO. It's not like it's a user-visible change.

…

On Tue, Apr 30, 2019 at 8:20 PM kkm (aka Kirill Katsnelson) < ***@***.***> wrote: There is also std::variant <https://en.cppreference.com/w/cpp/utility/variant>, also C++17. This thing allows only one of the specified types, like std::variant<string, int, float>. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3263 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAZFLO76ZYBDDFOFI4AAGI3PTDO5DANCNFSM4HIF6ZQQ> .

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.