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

[OUTDATED][WIP]: Make Taal and Taam produce identical (to floating point accuracy) results #253

Closed
wants to merge 56 commits into from

Conversation

sloede
Copy link
Member

@sloede sloede commented Oct 27, 2020

Closes #252.

Elixirs to check for results to match their TOML counterpart:

  • examples/1d/elixir_advection_amr.jl
  • examples/1d/elixir_advection_amr_nonperiodic.jl
  • examples/1d/elixir_advection_basic.jl
  • examples/1d/elixir_euler_blast_wave_shockcapturing.jl
  • examples/1d/elixir_euler_ec.jl
  • examples/1d/elixir_euler_nonperiodic.jl
  • examples/1d/elixir_euler_source_terms.jl
  • examples/2d/elixir_advection_amr.jl
  • examples/2d/elixir_advection_basic.jl
  • examples/2d/elixir_advection_mortar.jl
  • examples/2d/elixir_euler_blast_wave_shockcapturing.jl
  • examples/2d/elixir_euler_blast_wave_shockcapturing_amr.jl
  • examples/2d/elixir_euler_blob_mortar_shockcapturing.jl
  • examples/2d/elixir_euler_blob_shockcapturing_amr.jl
  • examples/2d/elixir_euler_density_wave.jl
  • examples/2d/elixir_euler_ec.jl
  • examples/2d/elixir_euler_khi_shockcapturing.jl
  • examples/2d/elixir_euler_khi_shockcapturing_amr.jl not yet tested
  • examples/2d/elixir_euler_nonperiodic.jl
  • examples/2d/elixir_euler_sedov_blast_wave_shockcapturing_amr.jl More adaptations for Taal/Taam #268
  • examples/2d/elixir_euler_source_terms.jl
  • examples/2d/elixir_euler_vortex.jl
  • examples/2d/elixir_euler_vortex_mortar.jl
  • examples/2d/elixir_euler_vortex_mortar_shockcapturing.jl
  • examples/2d/elixir_euler_vortex_mortar_split.jl
  • examples/2d/elixir_euler_vortex_shockcapturing.jl
  • examples/2d/elixir_euler_weak_blast_wave_shockcapturing.jl
  • examples/2d/elixir_eulergravity_jeans_instability.jl
  • examples/2d/elixir_eulergravity_sedov_blast_wave_shockcapturing_amr.jl
  • examples/2d/elixir_hyp_diff_harmonic_nonperiodic.jl
  • examples/2d/elixir_hyp_diff_llf.jl
  • examples/2d/elixir_hyp_diff_nonperiodic.jl
  • examples/2d/elixir_hyp_diff_upwind.jl
  • examples/2d/elixir_mhd_alfven_wave.jl
  • examples/2d/elixir_mhd_alfven_wave_mortar.jl
  • examples/2d/elixir_mhd_blast_wave_shockcapturing_amr.jl cannot be tested -> crashes
  • examples/2d/elixir_mhd_ec.jl
  • examples/2d/elixir_mhd_orszag_tang_shockcapturing_amr.jl
  • examples/2d/elixir_mhd_rotor_shockcapturing_amr.jl up to t_end=0.02 the differences to Taam are FP zero, while for 0.04 they are already O(1e-8)

Relevant only after merging dev:

@sloede sloede changed the title Have Taal and Taam produce identical (to floating point accuracy) results WIP: Have Taal and Taam produce identical (to floating point accuracy) results Oct 27, 2020
@ranocha ranocha added the taal label Oct 27, 2020
sloede and others added 3 commits October 27, 2020 12:12
@ranocha
Copy link
Member

ranocha commented Oct 27, 2020

The CFL computation in Taam was still different from Taal; I've adapted it for Euler 2D in b7ae3a1. Now, 2d/elixir_euler_ec.jl and parameters_euler_ec.toml both result in the same number of time steps when running till t_end = 1.0. I'll leave the remaining adaptations to you, @sloede

@andrewwinters5000
Copy link
Member

Locally I merged the shock capturing with nonconservative terms with this branch. The Orszag-Tang vortex is not reproducible there with Taam vs Taal

@sloede
Copy link
Member Author

sloede commented Oct 28, 2020

Locally I merged the shock capturing with nonconservative terms with this branch. The Orszag-Tang vortex is not reproducible there with Taam vs Taal

Hm, that's weird. But - wait, did you implement everything in Taal and Taam exactly identical in your branch? Cause if not, than that's bound to happen. If you did - well, then it is apparently not identical somewhere....

@sloede
Copy link
Member Author

sloede commented Oct 28, 2020

My plan for this branch is as follows: Finish the examples that are currently present here. Then update the tests such that all tests pass again. That way I know what breaks when merging in new stuff from dev. It's more work upfront, but I hope that this way no bugs introduced upstream (not that I expect you to add anything, just in case) do not slip through.

@andrewwinters5000
Copy link
Member

Hm, that's weird. But - wait, did you implement everything in Taal and Taam exactly identical in your branch? Cause if not, than that's bound to happen. If you did - well, then it is apparently not identical somewhere....

I am not sure. But I did check that if I deactivate AMR then the orszag-tang with the new shock capturing matches. So I think it might be that I set up the AMR resolutions differently or something

@sloede sloede changed the title WIP: Have Taal and Taam produce identical (to floating point accuracy) results WIP: Make Taal and Taam produce identical (to floating point accuracy) results Oct 29, 2020
@sloede sloede mentioned this pull request Oct 29, 2020
7 tasks
@sloede sloede changed the title WIP: Make Taal and Taam produce identical (to floating point accuracy) results [OUTDATED][WIP]: Make Taal and Taam produce identical (to floating point accuracy) results Nov 2, 2020
@sloede
Copy link
Member Author

sloede commented Nov 4, 2020

I think this can be closed. What do you think, @ranocha?

@ranocha
Copy link
Member

ranocha commented Nov 5, 2020

Yes (if you made sure that we've copied every significant and important change).

@sloede
Copy link
Member Author

sloede commented Nov 5, 2020

Alas, you're right. When closing this PR we should also delete the corresponding branch, but that's still useful for e.g., the fixes to the MHD fluxes. I'll leave this open then until we've merged #267.

@ranocha ranocha mentioned this pull request Nov 11, 2020
50 tasks
@ranocha
Copy link
Member

ranocha commented Nov 12, 2020

Can we close this now, @sloede?

@sloede
Copy link
Member Author

sloede commented Nov 12, 2020

Yes. Done.

@sloede sloede closed this Nov 12, 2020
@sloede
Copy link
Member Author

sloede commented Nov 12, 2020

Is it OK to also delete the corresponding branch? I think yes, but it's irreversible, so feel free to do it if you also think it's ok.

@ranocha ranocha deleted the msl/same-results-taam-taal branch November 12, 2020 17:05
@ranocha
Copy link
Member

ranocha commented Nov 12, 2020

I think so, too. To be safe, I pushed the branch also to my fork.

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

Successfully merging this pull request may close these issues.

3 participants