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

Wind test including radiation force + ieos = 17 + bug fixes #542

Merged
merged 14 commits into from
May 14, 2024

Conversation

lsiess
Copy link
Contributor

@lsiess lsiess commented May 6, 2024

Type of PR:
Bug fix / modification to existing code (fixes #540)

Description:
Include a new series of tests in the test_wind module :

  • check total internal and kinetic energy of the particles after the test
  • test wind problem when radiation force + Bowen opacity are included

Implement ieos = 17 to keep gamma constant but allow the mean molecular weight to change according to H2 formation

Bug fixes

  • case ieos = 5 was restricted to simulations with nucleation. It can now be used in any setup (same for ieos=17)
  • fix bug in temperature evaluation in calc_muGamma

Testing:
run simple wind models with ieos=5,17 to check that mu and gamma where not held constant

Did you run the bots? no

Did you update relevant documentation in the docs directory? no

@danieljprice
Copy link
Owner

this all looks good, remaining failure is just that the tolerance needs to be a bit higher for the internal energy:

 checking wind mass loss rate...........OK     [max err = 1.330E-16, tol = 2.220E-16]
 checking total internal energy.........FAILED [got  2.218E+02 should be   2.218E+02, err = 8.584E-15, tol = 5.000E-15]

Copy link
Owner

@danieljprice danieljprice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thinking of future maintenance it would be helpful to know where the numbers come from :)

call checkval(npart,12180,0,nfailed(3),'number of ejected particles')
call checkval(xyzmh_ptmass(15,1),1.591640703559762E-06,epsilon(0.),nfailed(4),'wind mass loss rate')
if (testcyl) then
call checkval(eint,3.360686893182378E+03,eps_sum,nfailed(5),'total internal energy')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would really help here to know where these numbers come from, e.g. if we improve things how do we know if it is better? Are these just initial values, or are they from some semi-analytic solution?

Copy link
Contributor Author

@lsiess lsiess May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no analytical solution and these numbers are associated to a non converged simulation. To reach convergence on the analytical solution would require very high resolution and very long integration time.
This is just a check that the code consistently gives the same numbers.
If the timestepping or the discretisation of the equations (e..g 2nd or fourth order) change the numbers will be completely different (relative error can exceed several percents).
The tests on Eint and Ekin were not included in the previous version of test_wind.

call checkval(eint,3.367417540822784E+03,eps_sum,nfailed(5),'total internal energy')
call checkval(ekin,5.524867074648306E+01,eps_sum,nfailed(6),'total kinetic energy')
else!if (test)
call checkval(eint,3.179016341424608E+03,eps_sum,nfailed(5),'total internal energy')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment, can these be calculated from first principles or are they just regression tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they are just regression tests

eint = sum(vxyzu(4,1:npart))
ekin = sqrt(sum(vxyzu(1,1:npart)**2+vxyzu(2,1:npart)**2+vxyzu(3,1:npart)**2))
if (vb) print '(5(1x,es22.15),i8)',xyzmh_ptmass(4,1),xyzmh_ptmass(7,1),xyzmh_ptmass(15,1),eint,ekin,npart
call checkval(xyzmh_ptmass(4,1),1.199987815414834E+00,epsilon(0.),nfailed(1),'sink particle mass')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this m_init + m_inj?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be m_ini-m_inj

! check 1D wind profile
i = size(trvurho_1D(1,:))
!print '((5(1x,es22.15)))',trvurho_1D(:,i),massoftype(igas)
call checkval(trvurho_1D(2,i),7.058624412798283E+13,epsilon(0.),nfailed(2),'outer wind radius')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment with values here, can they be computed or are these just regression tests also?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

regression tests

call checkval(xyzmh_ptmass(4,1),1.199987894518367E+00,epsilon(0.),nfailed(6),'sink particle mass')
call checkval(xyzmh_ptmass(7,1),0.,epsilon(0.),nfailed(7),'mass accreted')
call checkval(npart,12180,0,nfailed(8),'number of ejected particles')
call checkval(xyzmh_ptmass(15,1),1.591640703559762E-06,epsilon(0.),nfailed(9),'wind mass loss rate')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this prescribed somewhere?

Copy link
Contributor Author

@lsiess lsiess May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

xyzmh_ptmass(15) is the mass loss rate. It is defined in the .in file. Don't know why I checked this variable. It is useless.

@danieljprice danieljprice merged commit a7e5c6a into danieljprice:master May 14, 2024
180 checks passed
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.

Need tests for sink_radiation functionality
2 participants