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

iip vs oop iteration test #179

Merged
merged 3 commits into from
Apr 23, 2023
Merged

Conversation

yash2798
Copy link
Member

No description provided.

@yash2798 yash2798 changed the title iip vs oop iteration test WIP: iip vs oop iteration test Apr 19, 2023
@codecov
Copy link

codecov bot commented Apr 19, 2023

Codecov Report

Merging #179 (99fc63f) into master (c1267c8) will increase coverage by 0.15%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #179      +/-   ##
==========================================
+ Coverage   92.92%   93.08%   +0.15%     
==========================================
  Files           7        7              
  Lines         636      636              
==========================================
+ Hits          591      592       +1     
+ Misses         45       44       -1     
Impacted Files Coverage Δ
src/trustRegion.jl 97.54% <ø> (+0.40%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ChrisRackauckas
Copy link
Member

I don't understand what you're doing here. Just run the code with maxiters = 1,2,3,4,5 and then check that the results are the same between the two modes?

@yash2798
Copy link
Member Author

I don't understand what you're doing here. Just run the code with maxiters = 1,2,3,4,5 and then check that the results are the same between the two modes?

Yeah, but I was thinking maybe this would be beneficial when we benchmark different schemes later, like to see how different methods converge differently. So, instead of doing maxiters=1,2,3... again and again, we can just simply store the values? Does this make sense?

@ChrisRackauckas
Copy link
Member

No because this will increase the memory requirement of the method. We shouldn't add cruft to the method for the benchmarking/testing purpose here when we don't need to.

@yash2798
Copy link
Member Author

No because this will increase the memory requirement of the method. We shouldn't add cruft to the method for the benchmarking/testing purpose here when we don't need to.

okay sure got it, i'll just add the test then

@yash2798
Copy link
Member Author

these tests pass. maybe this can be merged and i can add tests for more complex functions in the next PRs?

@yash2798 yash2798 changed the title WIP: iip vs oop iteration test iip vs oop iteration test Apr 23, 2023
@ChrisRackauckas ChrisRackauckas merged commit 615d895 into SciML:master Apr 23, 2023
@yash2798 yash2798 deleted the ys/tr_test branch April 25, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants