Closed bartvanerp closed 9 months ago
ping @bvdmitri :)
@bartvanerp Solution approach is good! I've changed some minor stuff, simple tests would be nice
Attention: 6 lines
in your changes are missing coverage. Please review.
Comparison is base (
28d5297
) 80.09% compared to head (c37ad05
) 81.26%.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Thanks @bvdmitri! @Chengfeng-Jia would you be available to test the functionality?
I will write some tests for that.
Hi, guys, I wrote some tests for this improvement. However, I added some new models to the existing test code. If you think it's unnecessary please let me know.
Thanks @Chengfeng-Jia! In my opinion the tests look good. I moved everything to test_inference.jl
, as we are specifically testing the inference()
function, but aside from that, excellent job! @bvdmitri ready for review
Hi @Chengfeng-Jia, I made a start with #104. I think it has mostly been resolved, but it lacks tests. If @bvdmitri agrees with the solution approach, we can start writing those.
@bvdmitri Is the proposed solution okay? I added the warn argument also in the
ModelOptions
struct, such that we can use this upon model creation.