Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Errors should be given in percentage terms #108

Closed
pgkirsch opened this issue Jul 28, 2021 · 7 comments
Closed

Errors should be given in percentage terms #108

pgkirsch opened this issue Jul 28, 2021 · 7 comments
Labels
Milestone

Comments

@pgkirsch
Copy link
Contributor

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

@pgkirsch pgkirsch added the bug label Jul 28, 2021
@pgkirsch pgkirsch added this to the v0.2 milestone Jul 28, 2021
@whoburg
Copy link
Collaborator

whoburg commented Jul 29, 2021

@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
Copy link
Contributor Author

@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
Copy link
Collaborator

whoburg commented Jul 29, 2021

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
Copy link
Contributor Author

pgkirsch commented Jul 29, 2021

(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
-----
  1. 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
Copy link
Collaborator

whoburg commented Jul 29, 2021

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
Copy link
Contributor Author

pgkirsch commented Jul 29, 2021

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 added a commit that referenced this issue Aug 9, 2021
* rename fit.error to fit.errors and add log error
* fit.error is a float (=fit.errors["rms_rel"])
* add a note explaining distinction in tutorial
* also fix the overloaded use of `f` in tutorial
pgkirsch added a commit that referenced this issue Aug 15, 2021
@pgkirsch
Copy link
Contributor Author

Closed by #105

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants