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

test: more gradient optimizer tests #1217

Merged

Conversation

novikov-alexander
Copy link
Contributor

No description provided.

@novikov-alexander novikov-alexander changed the title tests: more gradient optimizer tests test: more gradient optimizer tests Nov 13, 2023
Tuple.Create(grads1, var1 as IVariableV1)
};
var sgd_op = tf.train.GradientDescentOptimizer(3.0f)
.apply_gradients(grads_and_vars, global_step: global_step);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AsakusaRinne why does apply_gradiens take System.Tuple while zip produces (T1, T2). Would it be better to replace or extend apply_gradients interface to support valuetuple as well?

@novikov-alexander novikov-alexander force-pushed the alnovi/gradient_more_tests branch 2 times, most recently from 9308d67 to 1f3f39e Compare November 13, 2023 13:09
// Validate updated params
var np_pred = 1.0 * 4.0 + 2.0 * 5.0 + 3.0;
var np_grad = 2 * np_pred;
self.assertAllCloseAccordingToType(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Wanglongzhi2001 Hm, now it calculates but the test doesn't pass. However, the code corresponds to TensorFlow original test. I have to check math there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There just was a small typo, but I didn't have time to debug it :-D

@novikov-alexander
Copy link
Contributor Author

novikov-alexander commented Jun 18, 2024

Hi @AsakusaRinne @Wanglongzhi2001 @Oceania2018
I fixed the mistake. Please review when you have bandwidth

@Oceania2018 Oceania2018 merged commit 0392027 into SciSharp:master Jun 19, 2024
3 checks passed
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.

3 participants