beanmachine icon indicating copy to clipboard operation
beanmachine copied to clipboard

Marginal 2D diagnostic tool

Open ndmlny-qs opened this issue 1 year ago • 5 comments

Motivation

Continued work on the diagnostics tools.

Changes proposed

  • Removed items in the pointStatistic.ts module and placed them in other modules for better structure. Updated all modules that needed to be updated due to the move.
  • Moved the method updateAxisLabel to a new module called utils/plottingUtils.ts for reuse.
  • Significantly reduced the complexity in webpack.config.js and tsconfig.json.
  • Updated the viz.py file to include the new marginal 2d tool.
  • Added the marginal 2d tool.

Test Plan

Just visual inspection for now, test to come soon.

Types of changes

  • [ ] Docs change / refactoring / dependency upgrade
  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • [x] My code follows the code style of this project.
  • [ ] My change requires a change to the documentation.
  • [ ] I have updated the documentation accordingly.
  • [x] I have read the CONTRIBUTING document.
  • [ ] I have added tests to cover my changes.
  • [ ] All new and existing tests passed.
  • [x] The title of my pull request is a short description of the requested changes.

ndmlny-qs avatar Nov 02 '22 22:11 ndmlny-qs

@horizon-blue has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Nov 03 '22 15:11 facebook-github-bot

Looks like there're some version updates in yarn.lock but there's no change in package.json. Are those updates necessary? I'm asking because due to security reason, we have to check in third-party codes into our internal codebase and load the packages from there instead of downloading them from Yarn/NPM each time we build the library. So every time we introduce a new dependency, we will have to import and check in a bunch of new packages.

Our internal jobs currently fails because it cannot find the node modules that are being used, so I just want to check with you to see if the versions that were in yarn.lock are sufficient for what we need? If so, then it will save me from the hassle of importing a bunch of new packages every time there's a change to our diagnostic module :)

horizon-blue avatar Nov 03 '22 22:11 horizon-blue

This is totally my mistake. I was trying out Bokeh 3.0 and did an update with yarn, which is why the lock file changed. I completely forgot to recheckout the lock file and not commit the changes. I'll revert now those changes.

ndmlny-qs avatar Nov 04 '22 00:11 ndmlny-qs

@ndmlny-qs has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot avatar Nov 04 '22 00:11 facebook-github-bot

@horizon-blue has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Nov 04 '22 00:11 facebook-github-bot