convexengineering / gpfit

Fit posynomials to data
http://gpfit.readthedocs.io/en/latest/
MIT License
10 stars 7 forks source link

Errors should be given in percentage terms #108

Closed pgkirsch closed 2 years ago

pgkirsch commented 3 years ago

Errors seem to be printed as percentages but actually calculated as absolute values

whoburg commented 3 years ago

@pgkirsch it looks like we're taking an exp before computing error? I think error should be in log space -- screenshot from the paper below to help with discussion

Screen Shot 2021-07-28 at 11 53 49 PM
pgkirsch commented 3 years ago

@whoburg Ah that's what I was looking for! (I vaguely remembered this being in the paper but somehow missed it.)

But why be satisfied with approximate error when it's so easy to get the exact error in original space? And if we are going to compute max error we need to compute it anyway...

whoburg commented 3 years ago

I agree it's easy to compute what any of these errors are, but there's only one of them that our fitting algorithm is actively minimizing (it's minimizing RMS error on the transformed data. The point of equation (42) is that this minimization should also result in low relative error on w.

pgkirsch commented 3 years ago

(Copying over from other discussion because it's easier to find here)

Ah fair point. Do you think any of these solutions would help add sufficient clarity?

  1. Make the heading of verbosity=1 output more explicit, i.e. instead of:
    Error
    -----

    it could be

    Error (in original space) # or log space
    -----
  2. Expand the self.error dict to include both types of error, i.e.
    self.error = {
    "transformed": {
        "rms": ...,
    },
    "untransformed": { # naming?
        "rms_rel": ...,
        "rms_abs": ...,
        "max_rel": ...,
        "max_abs": ...,
    }
    }

    But it's kinda cumbersome as a user to be like f.error["untransformed"]["rms_rel"]

  3. Similar but condensed:
    self.error = {
    "rms_log": ...,
    "rms_rel": ...,
    "rms_abs": ...,
    "max_rel": ...,
    "max_abs": ...,
    }
whoburg commented 3 years ago

Could choose one that we care the most about and call it self.error (it's a float), and then also provide a more complicated self.errors dict as you describe above

pgkirsch commented 3 years ago

I like that! My vote is for "rms_rel" (untransformed) if that would be ok with you. I think its the most meaningful error to an engineering-application end user.

I'll add a brief discussion of all this to the documentation.

pgkirsch commented 2 years ago

Closed by #105