modern-fortran / neural-fortran

A parallel framework for deep learning
MIT License
395 stars 82 forks source link

Proposition of API for the method `network % evaluate` #182

Closed jvdp1 closed 3 months ago

jvdp1 commented 4 months ago

Related to #179

milancurcic commented 3 months ago

I added example use of this to examples/dense_mnist.f90.

One challenge I found is that if we use the multiple metrics variant, we can't mix and match functions defined in nf_metrics and those in nf_loss (e.g. metrics=[mse(), corr()] is not allowed because an array constructor must have all types the same).

I think there may be a workaround by first staging metrics as class(metric_type), allocatable :: metrics(:) but I couldn't quite figured it out.

Even if it can't be done, it's a minor limitation IMO.

I'll add some tests too.

milancurcic commented 3 months ago

@jvdp1 actually, my challenge I mentioned above extends to even passing multiple metrics of the same parent type. Can you show me one example of invoking net % evaluate_batch_1d_metrics()?

milancurcic commented 3 months ago

I think once we add one example of passing multiple metrics (say, in a test program), we can merge this PR.

jvdp1 commented 3 months ago

One challenge I found is that if we use the multiple metrics variant, we can't mix and match functions defined in nf_metrics and those in nf_loss (e.g. metrics=[mse(), corr()] is not allowed because an array constructor must have all types the same).

Indeed, you are right. The same problem happens also with multiple metrics of the same parent type. So, it seems that evaluate_batch_1d_metrics is not possible and should be removed.

jvdp1 commented 3 months ago

A possible solution could be this one proposed by Brad. But it sounds to me a bit too complex for what we want to achieve.

milancurcic commented 3 months ago

Yes, I think this is the same approach that we use to pass an array of heterogeneous layers to a network. It's also overkill for the user to do this. Let's remove the multi-metrics variant then, at least until we hear anyone really needing this.

milancurcic commented 3 months ago

Thank you!