serialization for mcc #3906
serialization for mcc #3906
Conversation
kmatzen
force-pushed
the
kmatzen/mcc_serialization
branch
from
09daed0 to
8565a10
Compare
| void ColorCorrectionModel::write(FileStorage& fs) const | ||
| { | ||
| fs << "{" | ||
| << "ccm" << p->ccm | ||
| << "loss" << p->loss | ||
| << "src" << p->src | ||
| << "dist" << p->dist | ||
| << "cs_enum" << p->cs_enum | ||
| << "src_rgbl" << p->src_rgbl | ||
| << "dst_rgbl" << p->dst_rgbl | ||
| << "mask" << p->mask | ||
| << "ccm_type" << p->ccm_type | ||
| << "shape" << p->shape | ||
| << "linear" << *p->linear | ||
| << "distance" << p->distance | ||
| << "linear_type" << p->linear_type | ||
| << "weights" << p->weights | ||
| << "weights_list" << p->weights_list | ||
| << "ccm0" << p->ccm0 | ||
| << "gamma" << p->gamma | ||
| << "deg" << p->deg | ||
| << "saturated_threshold" << p->saturated_threshold | ||
| << "initial_method_type" << p->initial_method_type | ||
| << "weights_coeff" << p->weights_coeff | ||
| << "masked_len" << p->masked_len | ||
| << "max_count" << p->max_count | ||
| << "epsilon" << p->epsilon | ||
| << "dst_use_constcolor" << p->dst_use_constcolor; | ||
|
|
There was a problem hiding this comment.
Thank you for the contribution. The code looks good overall, but I feel we don’t need to write/save this many details.
I think keeping [ccm, loss, colorchart, ccm_type, gamma, deg, saturated-threshold, linear-type, distance-type] should be sufficient.
There was a problem hiding this comment.
That makes sense. My reasoning for serializing everything was (1) reloading the model in another process should enable all interfaces on the class to function as if it were the original and (2) if state inside the class is non-essential, then should it be stored in the class in the first place? Are some of the fields only cached state for the others?
There was a problem hiding this comment.
(1) For inference, only p->ccm, p->linear, p->cs, and p->shape (which can also be derived from ccm_type) are actually needed. You can see this directly in the infer() function in ccm.cpp.
(2) Most of the other fields are indeed just cached values to speed up CCM computation and aren't necessary for inference.
There was a problem hiding this comment.
You don't want to support use cases like a model being saved, loaded elsewhere, and then accessors like getWeights() being called?
There was a problem hiding this comment.
CCM calculation is not really something similar to a deep learning model where we want to save all the parameters and resume training.
The application is to just save the matrix and some other metadata for informative purpose and then use it for inference in an application
@asmorkalov what are your thoughts? Your comments are welcomed
I added serialization support for ColorCorrectionModel.
#3905
Pull Request Readiness Checklist
See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request
Patch to opencv_extra has the same branch name.