-
Notifications
You must be signed in to change notification settings - Fork 2
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
Improving the warnings when there is a gradient mismatch #27
Comments
Glad you find it useful!
Anything particular you have in mind here? Are talking about what the potential issues could be causing this? Or you mean more specifically exactly which function is casuing the issue? The former is easy enough to add, while hte latter is quite a bit more tricky (though technically not impossible) 😬 |
So either would be great functionality. I was thinking of something more limited which was just identifying which of our calls to make_turing_site was throwing this issue as we are running our benchmarks all together for many models. The simplest way to do that seems like it would be adding the model set as the first argument to make_turing_suite part of the message. Either of your suggestions might make that redundant. |
But given that |
No, I don't think it is a huge amount of effort but also seems like a lot of boiler-plate for users to add across a whole testing suite? My expectation was that this package was designed for that kind of deployment vs interactive running of a single suite and so this would make sense for a range of users as a feature but if that isn't the case I am happy to implement it in our wrapper and close this out. |
Firstly this is a great tool to have and I was pleased to find it!
Issue
We are using this here CDCgov/Rt-without-renewal#340 to help improve our code base and chase down issues.
We currently have a few gradient mismatches but the current approach to warnings (i.e. it doesn't indicate what part of our benchmark suite the issue is coming from) makes it a bit tricky/tedious to chase them down.
Potential solution
The addition of some information to the warning that helps localise the problem. I assume this would need to be information about the user-supplied model as information about any overarching suite organisation won't be available.
I am happy to have a go at implementing above suggestion or other suggestions if interested.
Alternative
This is an edge case due to introducing benchmarking all at once to an existing code base. If we instead had had it from the beginning or if we go through our code now and fix these things this should not be an issue as we add functionality iteratively going forward.
Alternatively there may be a feature here already we have missed that we could implement directly.
The text was updated successfully, but these errors were encountered: