ThomasDelteil commented on issue #11091: Symbolic .json file not compatible 
with .params file generated since MXNet 1.2
URL: 
https://github.com/apache/incubator-mxnet/issues/11091#issuecomment-397036787
 
 
   A more in-depth analysis written by @piiswrong about the cause of the issue 
and possible solutions:
   
   *Background*
   Gluon provides `save_params` API, which saves the model parameters (but not 
model definition) as a binary file ‘xxx.params’. It can be loaded back with API 
`load_params`. The saved file is intended to be opaque but you can load it with 
`mx.nd.load` and see the internal content (we don’t advertise this).
   Gluon also provides `export` API for saving a gluon model definition (.json) 
and parameters (.params), which can be loaded with MXNet Module or other 
language bindings.
   Gluon provides `SymbolBlock` API that can load .json model definition and 
.params parameter file. But an `import` helper for this functionality is 
missing.
    
   *The change*
   We changed the internal structure of the `.params` file saved by 
`save_params` to resolve a bug. Parameters saved by previous versions can still 
be loaded in new version.
    
   *The complaint from user*
   A user saved model definition and model parameters with `mx.sym.save_json` 
and `save_params`  following the straight dope book. Because the book doesn’t 
show how to load it back, customer invented a hacky way to load it into 
SymbolBlock with load_params. User's code broke after upgrading from 1.1 to 1.2.
    
   *The cause*
   Customer’s hack depended on internal similarities between .params files 
saved by `save_params` and `export`. After the change of `save_params` format, 
this hack stopped working.
    
   *Faults on our part*
   - The straight dope book should have recommended saving model definition 
with `export` instead of `mx.sym.save_json` and `save_params`.
   - We should have provided an `import` utility so that user doesn’t need to 
invent their own hacks to load model definition into SymbolBlock.
   - (?) We changed the file format saved by `save_params`. Although it is 
intended as an opaque binary whose format is not defined in documentation, some 
customers could be depending on undefined behavior.
    
   *Solutions*
   1. Revert `save_params` to previous format. Add new API `save_parameters` 
for new format.
   
   **Pros**
   - Customer relying on internal format of `save_params` won’t see breakage.
   
   **Cons**
   - All users need to manually migrate to new API `save_parameters`
   - Having both `save_params` and `save_parameters` could be confusing.
   
   <br>
   <br>
   2. Issue warnings and error messages to instruct users to move to `export` 
and `import`, and stop depending on undefined behavior
   
   **Pros**
   - Most users won’t see breakage and won’t need to do anything.
   
   **Cons**
   - Users depending on `save_params`’s internal format will see breakage.
    
   For both solutions we can add more documentation and helper API to minimize 
impact.
    
   
   Current open PRs related to this issue: 
https://github.com/apache/incubator-mxnet/pull/11236 
https://github.com/apache/incubator-mxnet/pull/11127 
https://github.com/apache/incubator-mxnet/pull/11210

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to