-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
PINNErrorVsTime Benchmark Updates #1159
base: master
Are you sure you want to change the base?
Conversation
@ChrisRackauckas I got an error on running the iterations which said that the maxiters are less than 1000 so I set all maxiters to 1100. Actually the decision was a bit arbitrary but is that a good number ?? |
https://docs.sciml.ai/SciMLBenchmarksOutput/v0.5/PINNErrorsVsTime/diffusion_et/ It's supposed to just show the error over time and then get cutoff. I don't see why making it longer would help. |
Yes. Actually I set it to that number just to get rid of that error |
Wait what's the error? |
the error went off on running again |
what error? |
maxiters should be a number greater than 1000 |
can you please just show the error... |
|
I see, that's for the sampling algorithm. You should only need that on Cuhre? |
Yes. But as Cuhre was the first one in the line I thought setting to 1100 just for it would not solve the problem, so I set it to 1100 for all of them |
The CI has passed here. And all the code seems to run perfectly. Can you please review ?? |
@ArnoStrouwen SciML/Integrals.jl#124 can you remind me what the purpose behind this was? |
I don't remember myself, but that PR links to: |
Uninitialized memory in the original C: giordano/Cuba.jl#12 (comment) fantastic stuff numerical community, that's your classic method that everyone says when they say "all of the old stuff is robust" 😅 |
Can you force latest majors and make sure the manifest resolves? |
I bump forced the latest versions and resolved manifests but initially there were a lot of version conflicts. I removed IntegralsCuba and IntegralsCubature for a while to resolve them. The manifest resolved but adding both of them back again poses some more version conflicts |
Can you share the resolution errors? |
@ChrisRackauckas These are the resolution errors that occur |
Oh those were turned into extensions. Change |
Sure !! 🫡 |
Should I set it to 1000+ ?? |
|
yes |
@ChrisRackauckas I got a few questions to ask for the diffusion_et.jmd and hamilton_jacobi.jmd in diffusion_et.jmd on running the code, I get this error Where this error comes from, it didn't occur before changing it to Cuba and Cubature |
What is safr_get_device? There is get_device from MLDataDevices which is implemented for all backend including CPU |
It's from you in NeuralPDE: |
Is UnknownDevice new? I don't think I've seen that. |
Oh that one is a stopgap solution for some of the Any issues that were coming up inside NeuralPDE
generally this doesn't get exposed to the user. You can get it if you call |
So, What changes do we need here ?? or is it an issue needed to be addressed in some other repository ?? |
|
I think these are the only 2 errors remaining here |
Checklist
contributor guidelines, in particular the SciML Style Guide and
COLPRAC.
Additional context
Add any other context about the problem here.