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

Standalone derivative evaluation #374

Open
mbkumar opened this issue Oct 31, 2023 · 2 comments
Open

Standalone derivative evaluation #374

mbkumar opened this issue Oct 31, 2023 · 2 comments

Comments

@mbkumar
Copy link
Collaborator

mbkumar commented Oct 31, 2023

In the functions taylor_test1 and taylor_test2 in tests/geo/test_surface_objectives.py, the function f has to be called with input x before evaluating derivatives through calls to df.
Why are the calls to the function, f necessary before calling df?

@smiet
Copy link
Contributor

smiet commented Nov 1, 2023

I cannot find the cause but I can reproduce.

The tests that fail are:

======================================================================
ERROR: test_iotas_derivative (geo.test_surface_objectives.IotasTests) (label='Volume')
Taylor test for derivative of surface rotational transform wrt coil parameters
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 289, in test_iotas_derivative
    self.subtest_iotas_derivative(label)
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 308, in subtest_iotas_derivative
    taylor_test1(f, df, coeffs,
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 19, in taylor_test1
    dfx = df(x)@direction
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 306, in df
    return io.dJ()
  File "/home/smiet/code/simsopt/src/simsopt/_core/derivative.py", line 217, in _derivative_dec
    return func(self, *args, **kwargs)(self)
  File "/home/smiet/code/simsopt/src/simsopt/geo/surfaceobjectives.py", line 788, in dJ
    if self._dJ is None:
AttributeError: 'Iotas' object has no attribute '_dJ'

Same for: 
ERROR: test_iotas_derivative (geo.test_surface_objectives.IotasTests) (label='ToroidalFlux')


FAIL: test_toroidal_flux_partial_derivatives_wrt_coils (geo.test_surface_objectives.ToroidalFluxTests) (surfacetype='SurfaceXYZFourier', stellsym=False)
Taylor test for partial derivative of toroidal flux with respect to surface coefficients
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 110, in test_toroidal_flux_partial_derivatives_wrt_coils
    self.subtest_toroidal_flux3(surfacetype, stellsym)
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 172, in subtest_toroidal_flux3
    taylor_test1(f, df, coeffs)
  File "/home/smiet/code/simsopt/tests/geo/test_surface_objectives.py", line 30, in taylor_test1
    assert err < 1e-9 or err < 0.3 * err_old
AssertionError

same for: 
FAIL: test_toroidal_flux_partial_derivatives_wrt_coils (geo.test_surface_objectives.ToroidalFluxTests) (surfacetype='SurfaceRZFourier', stellsym=False)
FAIL: test_toroidal_flux_partial_derivatives_wrt_coils (geo.test_surface_objectives.ToroidalFluxTests) (surfacetype='SurfaceXYZTensorFourier', stellsym=True)
FAIL: test_toroidal_flux_partial_derivatives_wrt_coils (geo.test_surface_objectives.ToroidalFluxTests) (surfacetype='SurfaceXYZTensorFourier', stellsym=False)

For the Iotas() class ERROR is caused by a bug in the Iotas class init, where Iotas._dJ is never initialized.
Fixed by changing its' recompute-bell to set it to None, as this bell is called in init

Fix is implemented in branch cbs/Iotas_hotfix: https://github.com/hiddenSymmetries/simsopt/tree/cbs/Iotas_hotfix

The taylor test fail is gnarlier IMO. subtest_toroidal_flux_3 tests a BiotSavart, and the simsoptpp part is hard for me to debug.
code:

    def subtest_toroidal_flux3(self, surfacetype, stellsym):
        curves, currents, ma = get_ncsx_data()
        nfp = 3
        coils = coils_via_symmetries(curves, currents, nfp, True)
        bs_tf = BiotSavart(coils)
        s = get_surface(surfacetype, stellsym)

        tf = ToroidalFlux(s, bs_tf)
        coeffs = bs_tf.x

        def f(dofs):
            bs_tf.x = dofs
            return tf.J()

        def df(dofs):
            bs_tf.x = dofs
            return tf.dJ_by_dcoils()(bs_tf)
        taylor_test1(f, df, coeffs)

Calling f(x) first must change something in the configuration of the 'BiotSavart` objects state leading to a slightly different evaluation. Hopefully someone with more cpp experience can look into this.

@mbkumar is merging the Iotas hotfix a priority or shall I leave my branch until we did into the other failures?
And Should we look into other cases where calling an objects derivative leads to a different result depending on whether its' value has been evaluated or not?

@mbkumar
Copy link
Collaborator Author

mbkumar commented Nov 1, 2023

@smiet
Thanks for looking into it. Let's fix the other failures also before we merge this. I'll look into the other failures soon.

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

No branches or pull requests

2 participants