Skip to content

Conversation

@samhatfield
Copy link
Collaborator

Builds on PR #331.

FSPGL_PROC is a procedure argument for the inverse transform which allows one to perform arbitrary calculations in spectral space. One passes in a subroutine name like any other argument, and this is passed down through the call stack to the place where it's actually called, in FSPGL_INT.

Currently we simply mark it as EXTERNAL, OPTIONAL. I think the EXTERNAL keyword is not encouraged anymore. The NAG compiler seems to reject it as well. Instead it's better to declare an explicit interface for the procedure argument and marking the argument as PROCEDURE(<interface name>). That's what I do here.

I hope you agree that this is much cleaner and more transparent. So far this change is NOT tested in the IFS though, so let me do that before merging.

@samhatfield samhatfield added the enhancement New feature or request label Nov 14, 2025
@wdeconinck
Copy link
Collaborator

I think it's good development but I have some questions.

  • Does this work with SPECTRAL_SPACE_FUNC_MOD conceptually being "internal", i.e. ectrans users don't need to use it?
  • Do we have a test that uses FSPGL_PROC, that would essentially stay unchanged with previous release?
  • Did you try this in an IFS experiment that actually uses the FSPGL_PROC?

@samhatfield
Copy link
Collaborator Author

  • Does this work with SPECTRAL_SPACE_FUNC_MOD conceptually being "internal", i.e. ectrans users don't need to use it?

Not really, this is something new. However, rather than introduce a new internal module, I could put the SPECTRAL_SPACE_FUNC interface in a header file in the include directory, like the externals. Then a user would #include spectral_space_func.h allowing them to define their own PROCEDURE(SPECTRAL_SPACE_FUNC) for passing to INV_TRANS etc. That's probably a better way of doing things.

  • Do we have a test that uses FSPGL_PROC, that would essentially stay unchanged with previous release?

We don't but I would like to write one once we have a good testing framework for INV_TRANS.

  • Did you try this in an IFS experiment that actually uses the FSPGL_PROC?

Not yet. I'll start with the T21 IFS tests and see how it goes. develop is now tested nightly as part of the CY50R3 CI so that's another thing to keep an eye on.

@wdeconinck
Copy link
Collaborator

If this is then an API change we cannot simply merge this without coordination.

@samhatfield
Copy link
Collaborator Author

samhatfield commented Nov 18, 2025

If this is then an API change we cannot simply merge this without coordination.

Yes understood, but how do you define API change? My intention was to keep backwards compatibility with the old-fashioned way (EXTERNAL like in the IFS), but I haven't tested that yet. Now that I think about it, I suppose it won't be backwards compatible. I will have a play around and report back.

In any case I don't think this will be merged soon @DJDavies2 so for now I'm afraid we won't be able to support the NAG compiler with ecTrans.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved-for-ci enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants