Skip to content

Conversation

jvdp1
Copy link
Collaborator

@jvdp1 jvdp1 commented Sep 21, 2025

Addition of the functions

  • get_optimizer_by_name
  • get_name for optimizer DT

I also set the function get_activation_by_name as pure

Copy link
Member

@milancurcic milancurcic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @jvdp1, the code looks fine.

What is the need for this? Is it for the user to be able to instantiate an optimizer by a string? Or does it add some new functionality that wasn't already possible?

@milancurcic
Copy link
Member

For getting the optimizer name, what do you think about setting a character(*), parameter :: name as a member to each concrete optimizer type, so then the name could be optimizer % name.

@jvdp1
Copy link
Collaborator Author

jvdp1 commented Sep 24, 2025

What is the need for this? Is it for the user to be able to instantiate an optimizer by a string?

Yes, it is. In my case, the user provides the name of the optimizer in a TOML file.

@milancurcic
Copy link
Member

That sounds good. And I realize now that it mirrors the same functionality for activation functions (get_activation_by_name), which was necessary for https://github.com/neural-fortran/nf-keras-hdf5.

Just let me know what you think about

what do you think about setting a character(*), parameter :: name as a member to each concrete optimizer type, so then the name could be optimizer % name.

and this can go ahead.

@jvdp1
Copy link
Collaborator Author

jvdp1 commented Sep 24, 2025

For getting the optimizer name, what do you think about setting a character(*), parameter :: name as a member to each concrete optimizer type, so then the name could be optimizer % name.

I think it is a good idea, and probably more efficient. I will implement it.

@milancurcic
Copy link
Member

I'm sorry for the confusion and back and forth. I now see that activation_function also has get_name, so your current approach mirrors it. In that case, actually better to stay consistent now, and we can discuss later if it should change in the style/approach. So, please merge this as is when ready.

@jvdp1 jvdp1 merged commit 3049393 into modern-fortran:main Sep 25, 2025
4 checks passed
@jvdp1 jvdp1 deleted the get_activation branch September 25, 2025 11:08
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

Successfully merging this pull request may close these issues.

2 participants