Skip to content

Conversation

@GiuseTripodi
Copy link
Contributor

@GiuseTripodi GiuseTripodi commented Dec 18, 2025

Summary

The 'model_summary()' method was resetting the model device from MPS or CUDA back to CPU after calling 'torchinfo.summary()'. By adding explicitly 'device' parameter to torchinfo.summar() the model stays on its original device throughout the summary operation.

Fixes #542

Describe your changes

Modified 'model_summary()' method by adding the 'device' parameter and setting it to the current device.

Checklist before assigning a reviewer (update as needed)

  • Self-review code
  • Ensure submission passes current tests
  • Add tests
  • Update relevant docs
  • Update changelog

Reviewer checklist

Please add anything you want reviewers to specifically focus/comment on.

  • Everything looks ok?

@rwood-97
Copy link
Collaborator

Hi, the tests are failing but I think this is a problem on my side.

I will fix this in the MapReader repo and then you will need to sync the changes from MapReader into your fork and then rebase your branch using git rebase main. You can then force push to update your branch on github using git push --force.

@rwood-97
Copy link
Collaborator

rwood-97 commented Dec 19, 2025

Hi @GiuseTripodi , I have (hopefully) now fixed the tests in the main MapReader repo, can you do the steps above and then let me know when you've done it and I'll review/approve the PR.

The changelog test checks whether you've updated the changelog. You'll need to add a Fixes subheading under the pre-releases section and add this fix and link to the PR. Docs are here.

Added device to the summary function to cleary define where the model and data needs to be stored.
@GiuseTripodi GiuseTripodi force-pushed the fix_model_summary_reset_device_#542 branch from 5c6dbfc to 03847df Compare December 19, 2025 16:34
@GiuseTripodi
Copy link
Contributor Author

Hi @rwood-97, I have done the steps above. Please let me know if there is anything else I should do.

---

## Pre-release
_Add new changes here_
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you re-add the _Add new changes here_ line so future contributors can still see where to add in the changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done it.

Copy link
Collaborator

@rwood-97 rwood-97 left a comment

Choose a reason for hiding this comment

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

Great, thanks for this.

I've added one comment re keeping the _Add new changes here_ line in the changelog. Otherwise looks good to me.

I've also noted that one of the tests is still failing but I think thats a separate issue (i.e. not for you to fix in this PR). I've created an issue on the repo to track it -> #576

@rwood-97 rwood-97 merged commit a849a3a into maps-as-data:main Jan 5, 2026
3 of 4 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.

Bug where model_summary resets device

2 participants