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

Enhance AssignResult and SamplingResult #1995

Merged
merged 5 commits into from
Jan 27, 2020

Conversation

Erotemic
Copy link
Contributor

Added random classmethods to SamplingResult and AssignResult.

These allow for the simple creation of "demo data" that can be use for testing, debugging, and potentially more (although I don't think these are as useful as random boxes might be).

Fix issue in SamplingResult.__init__

I've been hitting issues where sometimes gt_bboxes looks like its had its dimensions squashed. This causes an error when doing the index selection. Adding two checks fixes it.

Add rng as attribute of RandomSampler

To ensure that the new random methods could be seeded to consistently reproduce an issue if needed, I had to ensure that the RandomSampler class could be seeded. I used ensure_rng, which I added in a previous PR to accomplish this simplify.

Misc

I also added a to function to SamplingResult, where it moves the data onto a different device. I wrote this to test something that turned out not to be an issue, but I figure its a nice API to have so I left it in.

I also added info properties to SamplingResult and RandomSampler as well as implementing the __nice__ debugging string for SamplingResult.

I did end up using my ubelt library to accomplish some of these tasks. The core fix and the random clasmethods don't rely on ubelt, so if there is any resistance to adding a dependency I can remove the features that depend on ubelt and still have a functionally useful PR. However, I've been maintaining and keeping ubelt stable for ~2 years, it has 100% test coverage, it itself is fairly small, and its own dependencies are minimal.

@hellock
Copy link
Member

hellock commented Jan 26, 2020

Thanks for the PR as well as previous ones! ubelt is indeed an excellent util library, but it may be too early to introduce it since we only need two methods/classes of it. We can redefine __repr__ and __str__ to achieve the same goal of NiceRepr.

Add runtime dependency on ubelt (pending approval)

Fix issue in SamplingResult.__init__

Add rng as attribute of RandomSampler
@Erotemic Erotemic force-pushed the dev/enhance_sample_assign2 branch from 6811333 to e73f65d Compare January 26, 2020 18:38
@Erotemic
Copy link
Contributor Author

Sounds good. I removed ubelt as a dependency.

Instead of redefining __repr__ and __str__ (which imo would unnecessarily bloat the code) I simply copied the definition of NiceRepr into mmdet.utils. This removes the dependency on ubelt, but keeps the succinct and easy-to-modify __nice__ definition. It also prevents code redundancy in both SamplingResult and AssignResult.

I was also using ubelt.repr2 in this PR, which unfortunately is a bit too complex to simply copy-paste into a code base, so I simply wrote out some logic that produces a similar effect. Its less pretty than having a simple function that nicely handles nested representations, but it does prevent the extra dependency.

@hellock hellock merged commit 8457bba into open-mmlab:master Jan 27, 2020
mattdawkins added a commit to VIAME/mmdetection that referenced this pull request Mar 13, 2020
* origin/viame/master: (28 commits)
  Fix FPN upscale
  Extra compiler args
  VIAME-specific build parameters
  Bump version to 1.0.0 (open-mmlab#2029)
  Fix the incompatibility of the latest numpy and pycocotools (open-mmlab#2024)
  format configs with yapf (open-mmlab#2023)
  options for FCNMaskHead during testtime (open-mmlab#2013)
  Enhance AssignResult and SamplingResult (open-mmlab#1995)
  Fix typo activatation -> activation (open-mmlab#2007)
  Reorganize requirements, make albumentations optional (open-mmlab#1969)
  Encapsulate DCN into a ConvModule & Conv_layers (open-mmlab#1894)
  Code for Paper "Bridging the Gap Between Anchor-based and Anchor-free… (open-mmlab#1872)
  Non color images (open-mmlab#1976)
  Fix albu mask format bug (open-mmlab#1818)
  Fix CI by limiting the version of torchvision (open-mmlab#2005)
  Add ability to overwite existing module in Registry (open-mmlab#1982)
  bug for distributed training (open-mmlab#1985)
  Update Libra RetinaNet config with the latest code (open-mmlab#1975)
  Fix issue in refine_bboxes and add doctest (open-mmlab#1962)
  add link to official repo (open-mmlab#1971)
  ...
ioir123ju pushed a commit to ioir123ju/mmdetection that referenced this pull request Mar 30, 2020
* Enhance AssignResult and SamplingResult

Add runtime dependency on ubelt (pending approval)

Fix issue in SamplingResult.__init__

Add rng as attribute of RandomSampler

* fix linters

* remove ubelt

* Fix linters

* fix linters again
mike112223 pushed a commit to mike112223/mmdetection that referenced this pull request Aug 25, 2020
* Enhance AssignResult and SamplingResult

Add runtime dependency on ubelt (pending approval)

Fix issue in SamplingResult.__init__

Add rng as attribute of RandomSampler

* fix linters

* remove ubelt

* Fix linters

* fix linters again
FANGAreNotGnu pushed a commit to FANGAreNotGnu/mmdetection that referenced this pull request Oct 23, 2023
jben-hun pushed a commit to jben-hun/mmdetection that referenced this pull request Jan 10, 2025
* Enhance AssignResult and SamplingResult

Add runtime dependency on ubelt (pending approval)

Fix issue in SamplingResult.__init__

Add rng as attribute of RandomSampler

* fix linters

* remove ubelt

* Fix linters

* fix linters again
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