Simplified VC calculations

[kpentland]

In this PR, I've simplified the VC calculation process.

Primarily, this meant simplifying the way that plasma shape parameters are defined and used within the VC framework. This required some API changes and the Example09 notebook has been updated to reflect these changes. Hopefully they will come in handy.

[georgeholt1]

Thanks for this @kpentland! Definitely a lot simpler now.

I have a couple of questions/points. Some aren't related to what you've changed but I think they're still worth raising at this point.

• In apply_VC:
  

  freegsnke/freegsnke/virtual_circuits.py

  Lines 529 to 531 in 2dff80a

  	# calculate the targets
  	if not hasattr(self, "target_calculator"):
  	self.target_calculator = VC_object.target_calculator

  If self.target_calculator exists, might it be outdated, e.g. from a different VC calculation using the same object?

  Possible fix:

  old_target_values = VC_object.target_calculator(eq)
  # ... later ...
  new_target_values = VC_object.target_calculator(eq_new)

• In prepare_build_dIydI_j:

  freegsnke/freegsnke/virtual_circuits.py

  Lines 245 to 249 in 2dff80a

  	# relative norm of plasma current change
  	rel_ndIy_0 = np.linalg.norm(dIy_0) / self._nIy
  	# scale the starting_dI to match the target
  	final_dI = starting_dI * target_dIy / rel_ndIy_0

  Can the change in plasma current ever be zero here? If so, could be a division by zero error.

• self.solver is called quite a few times, but the solver attribute is only set when define_solver is called. I added checks for an attribute error to each of these.

• I think there's either a bug in calculate_VC or the documentation is wrong: the starting_dI argument is specified as a float in the docstring, but then is indexed in a couple of places as if it were an array:
  

  freegsnke/freegsnke/virtual_circuits.py

  Line 421 in 2dff80a

  f"{j}th coil ({coils[j]}) using initial current shift {starting_dI[j]}."

  freegsnke/freegsnke/virtual_circuits.py

  Line 423 in 2dff80a

  self.prepare_build_dIydI_j(j, coils, target_dIy, starting_dI[j])

  Based on the default behaviour (when None is passed), I assume this is always supposed to be an array?

[kpentland]

Thanks for the feedback @georgeholt1!

1. First point: agreed, not sure why that check was in there so I've removed and used the following as suggested:

old_target_values = VC_object.target_calculator(eq)
new_target_values = VC_object.target_calculator(eq_new)

2. It's very unlikely either of those two will ever be zero. For the first self._nIy to be zero, it would require your input equilibrium to have zero plasma current (which would be odd...). And for the second, rel_ndIy_0 to be zero, it means there is no change in the plasma current density between the initial equilibrium and the perturbed one. I've added a catch in there just in case, if it's triggered the user needs to increase the initial step size in the currents.

3. Nice spot, thanks for that.

4. Ah yes, the doc strings are wrong there thanks. I've updated and it should be an array. I've also changed the print statements for these outputs as they're a bit confusing.

[georgeholt1]

Awesome, thanks for the updates @kpentland

Will run the final tests now and merge