Skip to content

Conversation

@lmoneta
Copy link
Member

@lmoneta lmoneta commented Jun 12, 2024

This Pull request fixes the use case when the batch size is specified in RModel::Generate and the input model does not have a given batch size, but a parametric one

When input shape is parametric, give possibility to fix the batch size if provided by user in RModel::Generate
@lmoneta lmoneta force-pushed the fix_sofie_input_bs branch from bffabe6 to d99d652 Compare June 12, 2024 06:33
@lmoneta lmoneta requested a review from guitargeek June 12, 2024 06:34
@lmoneta lmoneta self-assigned this Jun 12, 2024
@vepadulano
Copy link
Member

Test and linked issue?

@github-actions
Copy link

github-actions bot commented Jun 12, 2024

Test Results

    12 files      12 suites   2d 19h 16m 52s ⏱️
 2 646 tests  2 643 ✅ 0 💤 3 ❌
30 025 runs  30 022 ✅ 0 💤 3 ❌

For more details on these failures, see this check.

Results for commit a7e4c2b.

♻️ This comment has been updated with latest results.

@lmoneta
Copy link
Member Author

lmoneta commented Jun 12, 2024

This is needed to fix root bench:
root-project/rootbench#239
It is a blocker for the summer student project. Thanks for reviewing it

lmoneta added 2 commits June 12, 2024 11:50
Fix for setting t he batch size when parsing a Keras model where the
input batch size is not defined
Add extra optional parameter in the Parse function to specify the batch size
…enerateModel

The GenerateModel function called for the GNN classes was having a default =1 for the batch size used when generating the update models.
We should use -1 to not force any specific batch size and use directly the given input shape
@dpiparo dpiparo self-requested a review June 12, 2024 13:44
Copy link
Member

@dpiparo dpiparo left a comment

Choose a reason for hiding this comment

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

LGTM only if all tests pass

@lmoneta lmoneta merged commit 45d08d4 into root-project:master Jun 13, 2024
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