Fix simulation file overwriting caused by same file name of forward and adjoint simulations

[txdai]

Following the bug fix in 2.7.4 for autograd, whenever a adjoint simulation is ran, it would have the same file name as the forward simulation file, causing overwrite. This is because both the forward and adjoint simulation calls this function with the same arguments

In more detail, currently when this function is called, the if statement in line 886 is never called because for the forward and adjoint simulation, its types are always "'simulation_type': ‘tidy3d_autograd’’, and the only way to differentiate between them is by using the “_adjoint” postfix in task_name.

I propose this fix here to add the postfix to the file name as well, to avoid the overwriting while being as less invasive as possible.

[yaugenst-flex]

Hi @txdai, good catch and thanks for submitting a fix, this is great! I guess you were referring to #1965?
It looks like sim_type is only set to autograd_{fwd,bwd} if not running local gradient computation (i.e. local_gradient=False). I think this intended behavior, maybe @tylerflex could confirm? In that case I think this fix is fine, otherwise we'd maybe need to have a look at our task naming.

[yaugenst-flex]

For clarity I'd prefer using os.path.splitext here, i.e.:

import os

...

name, ext = os.path.splitext(path)
path = name + "_adjoint" + ext

Functionally identical, but a bit easier to tell what's going on I think.

[txdai]

Sure this would also work, I was just reluctant to import another library

[yaugenst-flex]

Python stdlib is fine :)

[momchil-flex]

Thanks @txdai . We just had another discussion about this and to be very pedantic (although throwing away readability...) we reached this conclusion:

from os.path import basename, dirname, join
...
    if task_name.endswith("_adjoint"):
        path_parts = basename(path).split('.')
        path = join(dirname(path), path_parts[0] + "_adjoint." + ".".join(path_parts[1:]))

This is because we might have extensions like .hdf5.gz in principle. Not currently happening but to make this a bit more future-proof.

Do you want to modify your PR with this so we can merge it?

[yaugenst-flex]

oh and could you please make sure to run ruff on the PR once before committing, the tests are currently failing due to a lint error. if you use poetry, you can just poetry run pre-commit install on the repo, it'll run the linter automatically before the commit

[txdai]

I reformatted the code and changed to the suggested implementation. Thanks!

[tylerflex]

It looks like sim_type is only set to autograd_{fwd,bwd} if not running local gradient computation (i.e. local_gradient=False). I think this intended behavior, maybe @tylerflex could confirm? In that case I think this fix is fine, otherwise we'd maybe need to have a look at our task naming.

Yes this is the expected behavior. If running local_gradient=True, the server doesn't need to do anything special to handle fwd and adj simulations, they are just wrapped and handled by the front end. However with local_gradient=False, we need to know whether this was a forward or adjoint task to cache fields and do the gradient calculation, respectively

[momchil-flex]

Thanks - looks good now! I just need to ask one more thing, sorry - could you squash your two commits into 1? E.g. git rebase -i develop - a text file will open in your text editor with the two commits listed, replace pick on the second one with f (fixup), close the editor, and force push git push -f your branch here again.