You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mxnet.apache.org by GitBox <gi...@apache.org> on 2019/01/30 08:27:50 UTC

[GitHub] larroy edited a comment on issue #13964: Addresses comments in runtime feature discovery API

larroy edited a comment on issue #13964: Addresses comments in runtime feature discovery API
URL: https://github.com/apache/incubator-mxnet/pull/13964#issuecomment-458854880
 
 
   I have been thinking more about this and I would have the following proposal to address your concerns:
   
   1- Change the name of the implementation files and python module to `runtimeinfo.h` and mxnet.runtimeinfo in python. I would like to have the same name in the C api and bindings for consistency. And since we can't use libinfo we need to use something else.
   2- Leave the API calls as they are because the changes we discussed increase complexity either requiring memory allocation or getting the number of elements, etc. It's super simple to get the list of features and query a flag, these are two API calls. We can do with a single one but it requires what in my opinion is overengineering by returning an array of structs, each struct with a string. I think is better practice to have more simple API calls that do one thing than less calls that do many things. (A bad example of this is the posix file API for example, few calls, many ways to use it, complex). Interface segregation is usually a good practice and to me checking a simple boolean flag with no allocations is beautiful. Let me know if you feel strongly about this, or if this proposal would work for you to accept the PR.
   
   Pedro.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services