Revert "Merge branch 'restart-extra-defaults' into 'master'"

This reverts merge request !1533 (merged).

The logic of that was flawed, the space_extra_parts value should be 0 on restart and not 200 as found in the used parameter file. This is because the value is changed in space_init() when it will not be used.

added bug label

assigned to @matthieu

mentioned in merge request !1533 (merged)

requested review from @bvandenbroucke

Both Peter and I lost a bit of hair on this, so a third pair of eyes would not be too much. @bvandenbroucke if you could confirm this is indeed fine, that'd be great.

It depends on what the default value should be if you decided to remove the parameter from the parameter file.

If "parameter not present" means "use the SWIFT default", then this version is wrong. If it means "use the same value as before", then this version is correct.

space_extra_sparts, space_extra_gparts and space_extra_sinks are treated differently, since there is an extra condition in space_init() that changes them. If we decide "parameter not present" means the same as "use the SWIFT default", then that condition needs to be implemented in engine_config() as well.

I would actually argue that "use the SWIFT default" is the correct way to interpret the removal of a parameter from the parameter file, and that we should always apply the same extra conditions in engine_config() that are applied in space_init(). I don't see another way to correctly keep track of parameter changes.

So we need to copy the logic:

 /* Do we want any spare particles for on the fly creation?
     This condition should be the same than in engine_config.c */
  if (!(star_formation || with_sink) ||
      !swift_star_formation_model_creates_stars) {
    space_extra_sparts = 0;
    space_extra_gparts = 0;
    space_extra_sinks = 0;
  }

into engine_config(), that doesn't look nice, and I guess is what the commenter was thinking.

And not accept this MR of course.

So, we have that logic present already (missed that on first scan).

So in conclusion I think we should not accept this MR as the way things are now working is the same as when the program first started, that is if is set then we re-read that value from the parameter file (or the new value) and use it (not the default), and if it is not set then we continue to keep the default value stored in the param struct, but use the same value internally (0).

Going to close this unless someone really wants another behaviour.

Ok, I can be convinced that not having the parameter should mean "use the SWIFT default" rather than "continue with what you had".

If the code block is indeed present in both files, then we can go ahead.

closed