larroy commented on issue #15285: Graph dumper
URL: https://github.com/apache/incubator-mxnet/pull/15285#issuecomment-509815270
 
 
   @ptrendx not getting defensive, that was not my intention, let's not read 
between the lines. Feedback should be concrete an actionable for efficient use 
of everybody's time, that's why I indicated that if the feedback is unspecific 
and not clearly positive with this change I'm fine with closing the PR in 
interest of everyones time instead of letting it rot here or having a long 
discussion.
   
   I think there's many ways to implement something. I spent some effort 
implementing this to help us introspecting backward graphs, probably you could 
implement it in different ways, but this is the way I did it and I think is 
clean, concise, generic and has C++ unit tests.
   
   I think having a generic data structure like the one I introduced in this PR 
is better than using a non-generic structure into which implementation concerns 
are mixed with the data structure over and over in the implementation itself. 
This is in a similar but more concerning way to having a linked list by having 
the next pointer inside the object or as a generic class like std::list.  So 
for me having generic data structures make the code more maintainable. 
   
   For feedback to be useful it has also to be concrete, in this case I don't 
see the feedback to my code change proposal is concrete enough for me to take 
actions. Could you post an example of how would you propose to use indexed 
graph to do the same effect? Maybe that would help keep the conversation 
focused. It's also a debugging feature, so I don't feel specially passionate 
about defending my design decisions here. 
   
   Adding this functionality was requested by @apeforest @kshitij12345 and I 
while adding higher oder gradient.
   I spent some time adding unit tests and making it generic enough that can be 
re-used for different purposes, so please understand my surprise when there are 
unspecific questions like why do we need this or why introduce a small 
single-header data structure.
   
   Thanks for your comments.
   

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


With regards,
Apache Git Services

Reply via email to