chore: calculate only missing contour levels

[jobo322]

[cloudflare-workers-and-pages[bot]]

Deploying nmrium with    Cloudflare Pages

Latest commit:	f2d29f9
Status:	✅  Deploy successful!
Preview URL:	https://b5f095dc.nmrium.pages.dev
Branch Preview URL:	https://cache-contours-2d.nmrium.pages.dev

View logs

[jobo322]

I will try to point the problems when I have more time but I suggest you spend time learning React on your side because the things you did with the hooks are very wrong.

it were very expected, because I have not idea of react, but I would like show to luc, the behavior of contour plot cache while we wait for hamed, The time of contours generation reduce a lot when all the levels are cached.

[hamed-musallam]

@targos

Are you planning to refactor the code? If you're busy with something else, I can take care of the refactoring and fix his code.

[targos]

I'm not planning to refactor it, but I would like to review it when it's ready

[hamed-musallam]

@jobo322

I created a specialized context to handle contour calculations per nucleus https://github.com/cheminfo/nmrium/blob/f2d29f990ebda05dab0b950d1c96222db9827922/src/component/2d/ft/ContoursContext.tsx#L38-L53

This requires additional work:

1. Move the contour options from the display object within the spectrum object to view to avoid recalculating the contours when those options change (this can be done later once the reset work as expected)
2. Calculate all the contour levels once. Please review the drawContours function and make the necessary changes.

https://github.com/cheminfo/nmrium/blob/f2d29f990ebda05dab0b950d1c96222db9827922/src/component/2d/ft/ContoursContext.tsx#L23-L30

We have a custom hook, useContours, which returns an object where each key is a spectrum ID, and the value is an object with the following structure: { positive: { contours: [], timeout: boolean }, negative: { contours: [], timeout: boolean } }.

You need to create a new function to slice the contours (which include all the levels) based on the current zoom level https://github.com/cheminfo/nmrium/blob/f2d29f990ebda05dab0b950d1c96222db9827922/src/component/2d/ft/Contours.tsx#L80-L86

[jobo322]

It is not feasible for big files to calculate all the levels at once. is it possible to pass the boundaries to useContours?

[lpatiny]

It is not feasible for big files to calculate all the levels at once. is it possible to pass the boundaries to useContours?

@jobo322 Yes this is what I was afraid of. What is the maximal number of points in both directions that seems reasonable to you ?

This would make things much more complex because we can not simply cache the results as we were thinking before.

If we can cache currently the full analysis we should merge this PR once Michael has validated it. We will then think in another issue how to reduce the resolution and have 'quadrants' or something like that when data is too big. I may help on this and add some methods in ml-spectra-processing.

[lpatiny]

This comment just for documentation.

I did many some test with this file:

• https://github.com/mljs/conrec/tree/main/src/tests/data

That seems to me relatively big (2048 x 1024).

I also did a lot of benchmarks creating many contours very close to the noise:

https://github.com/mljs/conrec/tree/main/src/tests/data

This specific testcase generates 314'512'788 elements in lines array and generates 101 levels. The process on my mac m2 takes 8.4s.

In reality we should only calculate 10 levels at a time and it should not be such an extreme case. With the new approach we should therefore wait max 1s the first time it is being calculated.

[lpatiny]

@hamed-musallam Are you working on this PR ? Currently it does not work at all seems to me and is very slow. If it is easier we can close this PR and start over again because I would expect that it should not be such a difficult feature to implement to create an object cache that contains {key: level, value: contour}.

[hamed-musallam]

@hamed-musallam Are you working on this PR ? Currently it does not work at all seems to me and is very slow. If it is easier we can close this PR and start over again because I would expect that it should not be such a difficult feature to implement to create an object cache that contains {key: level, value: contour}.

No, but I believe Alejandro is working on this PR. It's better to keep it open to preserve the comments, and we can reset the branch to the main

[lpatiny]

You may also chedk:

• https://github.com/cheminfo/nmrium/pull/3243

[lpatiny]

@jobo322 Could you try to rebase this branch ?

[lpatiny]

Will be done in:

• https://github.com/cheminfo/nmrium/pull/3243