Add an optimizer library to score tracks

flyingtof commented 6 months ago

Summary by CodeRabbit

New Features
- Added a new GitHub link for flyingtof in the About modal.
- Introduced a computeScore method for optimized route scoring.
- Updated the optimize function in the optimizer library for enhanced performance.
Bug Fixes
- Corrected property access in PlannerElement from this.score.distance to this.score.distanceM.
Refactor
- Refactored scoring logic in PathElement for better readability and efficiency.
- Updated variable names in handleUpload method for clarity.
Documentation
- Added contribution guidelines in CONTRIBUTING.md.
Chores
- Updated various dependencies in package.json to their latest versions.
- Added new configurations for Babel, ESLint, and TypeScript in the optimizer library.
- Updated Docker configuration files for better environment setup.

flyingtof commented 6 months ago

Tell me if it's ok for you. I would like to stabilize this code because the more I modify it, the more the code for the next PR is less easy to rebase.

vicb commented 6 months ago

Tell me if it's ok for you. I would like to stabilize this code because the more I modify it, the more the code for the next PR is less easy to rebase.

Yep the best is to pause worked based on this until it gets merged. It doesn't have to be perfect to be merged but we should at least have a clear view about what is ok and what need to be updated later (i.e. TODOs).

I should be able to review tomorrow.

Thanks a lot!

flyingtof commented 6 months ago

@vicb do you have any idea why the tests fails on optimizer lib? I can not reproduce the problem locally (a nicer way to say WOMM ;-) )

flyingtof commented 6 months ago

@vicb do you have any idea why the tests fails on optimizer lib? I can not reproduce the problem locally (a nicer way to say WOMM ;-) )

issue created https://github.com/vicb/flyxc/issues/209

flyingtof commented 6 months ago

Replied to your comments. Hope you will find it OK.

flyingtof commented 6 months ago

I have a proposal for improvements when there is not enough points in the track to optimize. Can you have a look here: https://github.com/vicb/flyxc/blob/refactor/improve-optimizer-when-not-enough-points/libs/optimizer/src/lib/optimizer.ts ?

It ensures that when the track has not enough points, we send to the solver exactly the minimal number of point.
the added points are close to the original points:
- when 2 points: add 3 points near the first point
- when 3 points: add 1 point near the first point and 1 point near the second point
- when 4 points: add 1 point near the first point
it keep track of the proximity (a mapping of indices) of the added points with the original points so that retrieving the index in the original track does not require computations.

vicb commented 6 months ago

I have a proposal for improvements when there is not enough points in the track to optimize. Can you have a look here: https://github.com/vicb/flyxc/blob/refactor/improve-optimizer-when-not-enough-points/libs/optimizer/src/lib/optimizer.ts ?

It ensures that when the track has not enough points, we send to the solver exactly the minimal number of point.

the added points are close to the original points:

when 2 points: add 3 points near the first point

when 3 points: add 1 point near the first point and 1 point near the second point

when 4 points: add 1 point near the first point

it keep track of the proximity (a mapping of indices) of the added points with the original points so that retrieving the index in the original track does not require computations.

Is that getIndicesInScoringTrack ?

It's not clear to me what this functions does. It's not to comment what the fn does but when the code is tricky, it's good to comment how.

Notes:

You can get the link to a particular lines by clicking on the line number
Then press shift + other line number to get a range
Nested function (i.e. getIndicesInScoringTrack) should be avoided when possible. First because of readability and also because a fn object will have to be created each tilme the enclosing function is executed. If you really can not inline the fn you can move it before the enclosing function - or to a separate file if it's generic enough.

flyingtof commented 6 months ago

The SolverTrack class:

creates the appropriate track for the solver (by adding points if needed)
keep track (i.e. a mapping), when adding points, of the proximity link off the added points with the points on the original track.
permit to retrieve indices of solution points in the original track

vicb commented 6 months ago

Nested function (i.e. getIndicesInScoringTrack) should be avoided when possible. First because of readability and also because a fn object will have to be created each tilme the enclosing function is executed. If you really can not inline the fn you can move it before the enclosing function - or to a separate file if it's generic enough.

But getIndicesInScoringTrack is not a nested function, it's a member function of the SolverTrack class

Oops copy/paste error, I mean pushInResult

flyingtof commented 6 months ago

Oops copy/paste error, I mean pushInResult

Ok I pushed this improvement

vicb commented 6 months ago

Can you npm i and try again?

On Fri, May 31, 2024, 16:44 Christophe TARET @.***> wrote:

@.**** commented on this pull request.

In libs/optimizer/src/lib/optimizer.ts https://github.com/vicb/flyxc/pull/203#discussion_r1622530965:

@@ -231,7 +231,7 @@ const circuitMapping = { };

function toCircuitType(code: string): CircuitType {

const type = circuitMapping[code];

const type = circuitMapping[code as 'od' | 'tri' | 'fai' | 'oar'];

error occurred when running npm run dev

— Reply to this email directly, view it on GitHub https://github.com/vicb/flyxc/pull/203#discussion_r1622530965, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4X4SATWRBPZUBNRCC7E3ZFCEDJAVCNFSM6AAAAABIH3EPECVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJRGAZTGNBRGY . You are receiving this because you were mentioned.Message ID: @.***>

flyingtof commented 6 months ago

Can you npm i and try again?

ok. It works now.

vicb commented 6 months ago

Latest TS release has improved inference a lot See https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-4.html

On Fri, May 31, 2024 at 5:17 PM Christophe TARET @.***> wrote:

Can you npm i and try again? … <#m-2175490786768393315> On Fri, May 31, 2024, 16:44 Christophe TARET @.*> wrote: @.* commented on this pull request. ------------------------------ In libs/optimizer/src/lib/optimizer.ts <#203 (comment) https://github.com/vicb/flyxc/pull/203#discussion_r1622530965>: > @@ -231,7 +231,7 @@ const circuitMapping = { }; function toCircuitType(code: string): CircuitType { - const type = circuitMapping[code]; + const type = circuitMapping[code as 'od' | 'tri' | 'fai' | 'oar']; error occurred when running npm run dev — Reply to this email directly, view it on GitHub <#203 (comment) https://github.com/vicb/flyxc/pull/203#discussion_r1622530965>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4X4SATWRBPZUBNRCC7E3ZFCEDJAVCNFSM6AAAAABIH3EPECVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJRGAZTGNBRGY . You are receiving this because you were mentioned.Message ID: @*.***>

ok. It works now.

— Reply to this email directly, view it on GitHub https://github.com/vicb/flyxc/pull/203#issuecomment-2142484604, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4X4XMAZMUYPEXYUFG3BDZFCIABAVCNFSM6AAAAABIH3EPECVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBSGQ4DINRQGQ . You are receiving this because you were mentioned.Message ID: @.***>

vicb commented 6 months ago

The issue with the hack for 3d is:

it breaks the routing
it doesn't fix the root cause and the bug can still hit back in a future update on other part of the code

So I'm -1 on using this commit. The correct fix is for igc-xc-score to move away from using the collections package which pollutes the global namespace. See mmomtchev/igc-xc-score#234.

flyingtof commented 6 months ago

The issue with the hack for 3d is:

it breaks the routing

it doesn't fix the root cause and the bug can still hit back in a future update on other part of the code

So I'm -1 on using this commit. The correct fix is for igc-xc-score to move away from using the collections package which pollutes the global namespace. See mmomtchev/igc-xc-score#234.

I agree that this hack, as almost every hack, has drawbacks. It permit to have something that works for the user vs an application that can't ne used. I agree that it introduce a technical debt. I agree that it is a temporary solution, but is easy to revert when root cause disappear. But when switching to 3D mode, why does the application loads a library that

it has already loaded somewhere
it actually even does not need in 3D mode
require modifications in that library to work in flyxc context. Does it means that the problem may occur again when using other libraries? I think that the root cause is more in this loading mechanism. But that's beyond my knowledge to solve such an issue.

vicb commented 6 months ago

it has already loaded somewhere

it actually even does not need in 3D mode

Switching to 3D mode does not reload igc-xc-score (I guess it is what you mean). The correct way to think about it is that it doesn't unload it when switching to 3D. What flyxc does in this case is that it removes the 2D map and replaces it with the component holding the 3D map.

However when igc-xc-score was first loaded (in 2D mode) then the collections dep polluted the global namespace by changing builtin types prototype (see the issue on igc-xc-score).

It turns out that the changes done by collections are not compatible with the ArcGIS JS SDK used for the 3D map.

What your hack does is that it fully reload the page (instead of just swapping the 2D component for the 3D component). In this case the igc-xc-score lib is not loaded as it's a dep of the 2D only.

require modifications in that library to work in flyxc context. Does it means that the problem may occur again when using other libraries?

It's a problem with any library polluting the global namespace. Having a transitive dep on collections might cause other issue that we are not aware today and might cause issues later when any dep is updated.

collections is used only for Map and SortedSet in igc-xc-score, it should be easy to swap them for an implementation that does not pollute the global namespace.

coderabbitai[bot] commented 6 months ago

[!IMPORTANT]

Review skipped

More than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review.

95 files out of 152 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The recent changes encompass a wide range of updates, including refactoring scoring logic, introducing new methods and types, updating configurations, and enhancing documentation. Key updates include modifications to the PathElement and PlannerElement classes, restructuring league definitions, and updating scoring rules. Additionally, various configuration files and dependencies have been updated, and new testing and optimization functionalities have been introduced in the optimizer library.

Changes

File/Directory	Change Summary
`CONTRIBUTING.md`	Introduced guidelines for contributing to the project.
`apps/fxc-front/src/app/components/2d/path-element.ts`	Refactored scoring logic, introduced `computeScore` method, updated imports.
`apps/fxc-front/src/app/components/2d/planner-element.ts`	Updated property access from `this.score.distance` to `this.score.distanceM`.
`apps/fxc-front/src/app/components/ui/about-modal.ts`	Added a new GitHub link for `flyingtof`.
`apps/fxc-front/src/app/components/ui/main-menu.ts`	Updated `handleUpload` method to use `HTMLInputElement` type and renamed variables for clarity.
`apps/fxc-front/src/app/components/ui/pref-modal.ts`	Updated imports and usage to include `LEAGUE_CODES`.
`apps/fxc-front/src/app/logic/score/league/leagues.ts`	Restructured league definitions, introduced `LeagueCode` type, added `getScoringRuleName` function.
`apps/fxc-front/src/app/logic/score/scorer.ts`	Updated `Score` class properties and simplified `scoreTriangles` function.
`apps/fxc-front/src/app/redux/planner-slice.ts`	Updated `league` property type from `string` to `LeagueCode`.
`apps/fxc-front/vite.config.ts`	Added a new global object `{}` in the `define` section.
`docker/.env`	Added `COMPOSE_PROJECT_NAME` variable set to `flyxc`.
`docker/docker-compose.yml`	Defined services for launching Docker images with specific configurations.
`libs/optimizer/.babelrc`	Introduced Babel preset configuration using `@nx/js/babel`.
`libs/optimizer/.eslintrc.json`	Introduced ESLint configurations for TypeScript, JavaScript, and JSON files.
`libs/optimizer/README.md`	Described the `optimizer` library and its `optimize` function.
`libs/optimizer/jest.config.ts`	Configured Jest for testing the optimizer library.
`libs/optimizer/project.json`	Defined linting and testing configurations using Nx for the optimizer project.
`libs/optimizer/src/index.ts`	Introduced new exports for optimizer functions and types.
`libs/optimizer/src/lib/optimizer.spec.ts`	Added test cases for scoring rules in the optimizer module.
`libs/optimizer/src/lib/optimizer.ts`	Introduced functionality for optimizing flight tracks and defined related interfaces and functions.
`libs/optimizer/src/lib/scoring-rules.ts`	Introduced scoring rules for various competitions and exported related types and functions.
`libs/optimizer/src/lib/utils/create-segments.ts`	Added `createSegments` function to generate a track with segments.
`libs/optimizer/src/lib/utils/merge-tracks.ts`	Added `mergeTracks` function to merge multiple tracks.
`libs/optimizer/tsconfig.json`	Updated to extend a base configuration and reference additional configuration files.
`libs/optimizer/tsconfig.lib.json`	Introduced configuration settings for TypeScript compilation.
`libs/optimizer/tsconfig.spec.json`	Introduced configuration settings for TypeScript compilation specific to testing.
`nx.json`	Added new TypeScript compilation configuration with caching enabled.
`package.json`	Updated various dependencies and added new ones.
`tsconfig.base.json`	Added new path mapping for `@flyxc/optimizer` to `libs/optimizer/src/index.ts`.

Poem

In the code where logic thrives,
Paths and leagues take new drives.
Optimizer's wings now soar,
Scores refined, rules galore.
Docker's ship set to sail,
Contributions without fail.
CodeRabbit cheers, with joy so bright,
For changes made, a coder's delight! 🐇✨

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

vicb commented 6 months ago

I have pushed more changes to the PR.

igc-xc-score is now invoked from a worker so that we can workaround the global namespace issue more easily. We still need to drop the barrel file (index.ts) because importing the enum from there in fxc-front still causes the issue. That should be easy to do.

I have also added more info to the SocringResult.

Hopefully we can merge this tomorrow!

vicb commented 6 months ago

@coderabbitai generate interesting stats about this repository and render them as a table.

coderabbitai[bot] commented 6 months ago

[!TIP] For best results, initiate chat on the files or code changes.

@vicb, here are some interesting statistics about the repository:

Metric	Value
Number of Authors	7
Total Commits	678
Total Lines of Code	138,915
Number of Open Issues	30

These metrics provide a snapshot of the repository's current state, including contributions, codebase size, and ongoing issues.

Analysis chain

Scripts executed

The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Count the number of authors in the codebase. git shortlog -sn --all | wc -l # Count the total number of commits. git rev-list --all --count # Count the number of lines of code in the codebase. tokei --output json # Count the number of open issues using GitHub CLI. gh issue list --repo vicb/flyxc --state open --json number --jq 'length' ``` Length of output: 76545

vicb / flyXC