rice-solar-physics / HYDRAD

HYDrodynamics and RADiation Code for computing solutions to field-aligned hydrodynamic equations in coronal loops
MIT License
9 stars 4 forks source link

High precision in loop length input can lead to segmentation fault in initial conditions #82

Open wtbarnes opened 3 years ago

wtbarnes commented 3 years ago

I'm running into a case where running the initial conditions is throwing a segmentation fault and I have no idea why. I have two cases of very long loops with similar lengths, a loop with length 117 Mm (loop 36) and a loop with length 124 Mm (loop 47). The initial conditions code for loop 36 runs and produces a hydrostatic solution with no issue. Loop 47 throws a segmentation fault. The only difference between the two is the length and I cannot understand why the second would result in a segmentation fault and not the first. I have attached the two configured simulations here.

I have turned off both the gravity and expansion profiles for the initial conditions step as well so I cannot imagine that could be causing the problem either.

loop000036.tar.gz loop000047.tar.gz

wtbarnes commented 3 years ago

Just for completeness, here are the initial conditions for the simulation that does not throw a seg fault (loop 36). Everything seems as expected

image

sjbradshaw commented 3 years ago

Okay, the issue is something that I have come across before, but haven’t really looked into. It’s probably something to do with how the number of grid cells required for the initial conditions is calculated.

Anyhow, when you have a loop length to very high precision (e.g. 12450376730.92732, as in the Initial_Conditions/config/initial_conditions.cfg file) and a MIN_CELLS (in Initial_Conditons/config/config.h) that does not give a nice maximum grid cell width (e.g. 12450376730.92732 / 125) then problems can arise (though not always predictably, as you found with the other case).

The solution is to truncate the precision to which the loop length is given. E.g. 12450376730.92732 -> 1.24E10 and then MIN_CELLS becomes 124. This gives a ds_max of 1E8, which can be nicely refined to a small number (ds_min). There’s no need to give loop length to an arbitrarily high precision, anyway, so I wasn’t planning to spend a lot of time tracking this down and the fix is easy.

Cheers,

Steve

From: Will Barnes notifications@github.com Sent: Friday, December 11, 2020 10:54 AM To: rice-solar-physics/HYDRAD HYDRAD@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [rice-solar-physics/HYDRAD] Segmentation fault in initial conditions (#82)

Just for completeness, here are the initial conditions for the simulation that does not throw a seg fault (loop 36). Everything seems as expected

https://user-images.githubusercontent.com/8676141/101883900-e682ce80-3b65-11eb-8d62-65e23826cc54.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/HYDRAD/issues/82#issuecomment-743306652 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7XL3N37PW74R5QXBFTSUJFBTANCNFSM4UWJUAKA . https://github.com/notifications/beacon/ACC6C7W3JYUCV7MHUE4PYALSUJFBTA5CNFSM4UWJUAKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFRG7LHA.gif

wtbarnes commented 3 years ago

This appears to be a Linux-versus-Mac (and possibly Windows as well) issue. @jwreep and I have both confirmed that both of these configurations work on Mac, but not Linux. Additionally, it seems that configuring a nearly identical setup with the GUI does not lead to this seg fault, but doing the setup in Python (with pydrad) does. Why one succeeds and the other does not is beyond me.

wtbarnes commented 3 years ago

Ah okay. Thanks! That is very odd indeed. The high precision in the loop length is because these lengths are derived from a field extrapolation and thus are automatically generated. It is easy enough to just add some code to the configuration to truncate the precision.

wtbarnes commented 3 years ago

I've changed the name of the issue here and I'll leave it open just in case someone else (or my future self!) stumbles across this again.

jwreep commented 3 years ago

Based on Steve's response, have you tried simply setting in pydrad MIN_CELLS to 1 + the current number? It sounds like this might be a one-off issue due to truncating the precision.

sjbradshaw commented 3 years ago

I think it’s probably something like that.

I’ve posted a pydrad related issue, in the meantime. I may now have got pytest working by simply reinstalling. Does it do anything? Provide any output? It just runs for a bit and then exits for me (with no indication that it has done anything successfully or otherwise). However, trying to load and print the default configuration using the configure method still doesn’t work.

From: jwreep notifications@github.com Sent: Friday, December 11, 2020 1:20 PM To: rice-solar-physics/HYDRAD HYDRAD@noreply.github.com Cc: sjbradshaw stephen.bradshaw@rice.edu; Comment comment@noreply.github.com Subject: Re: [rice-solar-physics/HYDRAD] High precision in loop length input can lead to segmentation fault in initial conditions (#82)

Based on Steve's response, have you tried simply setting in pydrad MIN_CELLS to 1 + the current number? It sounds like this might be a one-off issue due to truncating the precision.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/HYDRAD/issues/82#issuecomment-743379844 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7WZRMCW6LBKZUKA5HTSUJWENANCNFSM4UWJUAKA . https://github.com/notifications/beacon/ACC6C7RRQYWTQUOXVY63TFTSUJWENA5CNFSM4UWJUAKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFRHRHBA.gif

wtbarnes commented 3 years ago

I'm now seeing this issue pop up again, this time for smaller loop lengths. In the pydrad configuration code, I've switched to writing the loop length out in scientific notation with 8 significant digits (I realize this is still far more than needed). I've also changed the calculation of the minimum number of cells to be ceil(L / min_grid_size) + 1 per Jeff's comment.

There are no issues with the longer loop lengths I mentioned previously. However, for a loop with length ~45 Mm, I'm now seeing the following runtime error,

Calculating initial hydrostatic conditions...

Peak heating range = 3.5481e-04 -> 3.6308e-04 erg cm^-3 s^-1

Optimum peak heating rate = 3.6039e-04 erg cm^-3 s^-1

Writing initial conditions file...

corrupted size vs. prev_size
[1]    21406 abort (core dumped)  ./Initial_Conditions.exe

Reducing the number of significant figures for the loop length in the configuration (even down to 2, e.g. 4.45e+9) just leads to the same segmentation fault as before. I've attached the configuration here.

loop000059.tar.gz

sjbradshaw commented 3 years ago

The problem is with MAX_CELLS in the config.h file. For some reason, you’re not allocating enough for the code to calculate the uniform grid at the highest spatial resolution for output. I usually have it default to some very large number, which should be sufficient to cover all cases (something like 30,000, I think). The code you sent me works fine with MAX_CELLS 5000. I haven’t checked to see what the smallest number it can be is.

From: Will Barnes notifications@github.com Sent: Saturday, December 12, 2020 12:44 PM To: rice-solar-physics/HYDRAD HYDRAD@noreply.github.com Cc: sjbradshaw stephen.bradshaw@rice.edu; Comment comment@noreply.github.com Subject: Re: [rice-solar-physics/HYDRAD] High precision in loop length input can lead to segmentation fault in initial conditions (#82)

I'm now seeing this issue pop up again, this time for smaller loop lengths. In the pydrad configuration code, I've switched to writing the loop length out in scientific notation with 8 significant digits (I realize this is still far more than needed). I've also changed the calculation of the minimum number of cells to be ceil(L / min_grid_size) + 1 per Jeff's comment.

There are no issues with the longer loop lengths I mentioned previously. However, for a loop with length ~45 Mm, I'm now seeing the following runtime error,

Calculating initial hydrostatic conditions...

Peak heating range = 3.5481e-04 -> 3.6308e-04 erg cm^-3 s^-1

Optimum peak heating rate = 3.6039e-04 erg cm^-3 s^-1

Writing initial conditions file...

corrupted size vs. prev_size [1] 21406 abort (core dumped) ./Initial_Conditions.exe

Reducing the number of significant figures for the loop length in the configuration (even down to 2, e.g. 4.45e+9) just leads to the same segmentation fault as before. I've attached the configuration here.

loop000059.tar.gz https://github.com/rice-solar-physics/HYDRAD/files/5683594/loop000059.tar.gz

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/HYDRAD/issues/82#issuecomment-743798471 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7QTDKVEPKZ6V65G6ZLSUO2X7ANCNFSM4UWJUAKA . https://github.com/notifications/beacon/ACC6C7QH3HP5VHNLNZQVS4DSUO2X7A5CNFSM4UWJUAKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFRKXNRY.gif

wtbarnes commented 3 years ago

I'm able to run the initial conditions setting MAX_CELLS to 5000 as well.

In pydrad, I'm calculating the maximum number of cells as floor(2^{RL} * L / ds_max) where RL is the refinement level,

https://github.com/rice-solar-physics/pydrad/blob/d4fdbf713e1dc1a6629308ba4adb5e0fbb7c3908/pydrad/configure/configure.py#L421-L439

Is this not correct? I've already added an option in the pydrad to just override this calculation by specifying maximum_cells in the configuration.

sjbradshaw commented 3 years ago

It should be… Maybe there’s some strange rounding error. Try comparing the number that calculation gives with the number of cells that are in the initial.amr file that’s produced.

From: Will Barnes notifications@github.com Sent: Saturday, December 12, 2020 5:01 PM To: rice-solar-physics/HYDRAD HYDRAD@noreply.github.com Cc: sjbradshaw stephen.bradshaw@rice.edu; Comment comment@noreply.github.com Subject: Re: [rice-solar-physics/HYDRAD] High precision in loop length input can lead to segmentation fault in initial conditions (#82)

I'm able to run the initial conditions setting MAX_CELLS to 5000 as well.

In pydrad, I'm calculating the maximum number of cells as floor(2^{RL} * L / ds_max) where RL is the refinement level,

https://github.com/rice-solar-physics/pydrad/blob/d4fdbf713e1dc1a6629308ba4adb5e0fbb7c3908/pydrad/configure/configure.py#L421-L439

Is this not correct? I've already added an option in the pydrad to just override this calculation by specifying maximum_cells in the configuration.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rice-solar-physics/HYDRAD/issues/82#issuecomment-743917249 , or unsubscribe https://github.com/notifications/unsubscribe-auth/ACC6C7XK5HBG4IUBCOCGJWTSUPY4JANCNFSM4UWJUAKA . https://github.com/notifications/beacon/ACC6C7WRVXDBBHYRDHKAO3DSUPY4JA5CNFSM4UWJUAKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOFRLUNQI.gif