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

segmentation fault in spectral cycles #114

Closed
kslong opened this issue Sep 14, 2014 · 8 comments
Closed

segmentation fault in spectral cycles #114

kslong opened this issue Sep 14, 2014 · 8 comments
Labels

Comments

@kslong
Copy link
Collaborator

kslong commented Sep 14, 2014

Python is now failing, with a segmtation fault, on a simple spherical polar model (at least on Macs). The failure is at the generation of detailed spectral files. The problem is not associated with mpi, as it fails with gcc as well as mpi at the same place, as far as I can tell.

The .pf file sphere_pol.pf is in bug_pf files on Dropbox

@kslong kslong added the medium label Sep 14, 2014
@kslong kslong added this to the Public Release milestone Sep 14, 2014
@jhmatthews
Copy link
Collaborator

This must be a recent change. I ran the fiducial and CV models in spherical polar just prior to the summer 2014 meeting with good results.

I think it could be related to the dynamic allocation of memory for the plasma structure. We do a dynamic allocation of the "scatters" member of the plasma structure, which is used when detailed spectra are created, so my guess is that's where it's going wrong. This is almost certainly related to my problems with py_wind which I haven't debugged yet. Making this high priority, as it needs to be sorted for that anyway.

@jhmatthews jhmatthews added high and removed medium labels Sep 14, 2014
@jhmatthews
Copy link
Collaborator

Hi Knox- this isn't specific to spherical polar, I don't think. I'm able to reproduce the error in cylindrical coordinates, and I think it's caused by this loop added to define_phot

  for (n=0;n<nphot_tot;n++){
      p[n].w_orig=p[n].w;
  }

The problem here is just that the photon structure only ever has size NPHOT i.e. the number of photons per cycle - i.e. it gets repopulated each time. So a simple fix of nphot_tot -> NPHOT here should solve the problem.

@jhmatthews
Copy link
Collaborator

while we're on the topic, why do we do all this stuff in python.c:

// rddoub ("photons_per_cycle", &photons_per_cycle);
// 140907 - ksl - Although photons_per_cycle is really an integer, read in as a double so it is easier for input
  x=100000;
  rddoub ("photons_per_cycle", &x);
  NPHOT = photons_per_cycle=x;  // For now set NPHOT to be be photons/cycle --> subcycles==1

  photons_per_cycle = (photons_per_cycle / NPHOT) * NPHOT;
  if (photons_per_cycle < NPHOT)
    photons_per_cycle = NPHOT;
  subcycles = photons_per_cycle / NPHOT;
  Log ("Photons_per_cycle adjusted to %d\n", photons_per_cycle);

#ifdef MPI_ON
  Log ("Photons per cycle per MPI task will be %d\n", photons_per_cycle/np_mpi_global);

  NPHOT/=np_mpi_global;
  photons_per_cycle/=np_mpi_global;
#endif

It seems to me the variable photons_per_cycle is redundant and can be replaced by NPHOT the global variable in all calls, much the same way we did in #106 . Agreed?

@jhmatthews
Copy link
Collaborator

I'd also be inclined to remove the subcycles variable, or at least clarify why one might want to use it.

@Higginbottom
Copy link
Collaborator

I think the original use of the sub cycles was to reduce the memory requirement of the code. So one split up the phot structure (which can be pretty big) into sub cycles, but could still have gazillions of photons per cycle...
This may well now be redundant....

@jhmatthews jhmatthews changed the title python failing in spherical polar coordiantes segmentation fault in spectral cycles Sep 16, 2014
@kslong
Copy link
Collaborator Author

kslong commented Sep 16, 2014

Nick is correct that the origin of this was a concern about memory requirements., and I also agree with those who would like to remove this complication. I suggest someone creating a separate issue regarding this and removing this complication from the code.
Knox

@jhmatthews
Copy link
Collaborator

Will do, closing this discussion.

@kslong
Copy link
Collaborator Author

kslong commented Nov 16, 2014

I confirmed that the segmentation fault that was occurring no longer occurs.

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

No branches or pull requests

3 participants