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

getRanges #49

Open
stropitek opened this issue Dec 6, 2016 · 9 comments
Open

getRanges #49

stropitek opened this issue Dec 6, 2016 · 9 comments

Comments

@stropitek
Copy link
Contributor

stropitek commented Dec 6, 2016

if (this.ranges) {

I don't think getRanges should return the cached value since the parameters to compute them could be different from when they were cached. Actually if the parameters to compute them are not passed to the constructor and immutable, I don't see any reason to keep ranges cached.

@lpatiny
Copy link
Member

lpatiny commented Dec 6, 2016

We should not forget that we need to be able to deal with ranges not associated with spectra (jcamp) because ranges may be created from a prediction for example.

Please comment what is the best as syntax and what is the current state :

var ranges = SD.predict1H (molfile);
var spectrum1H = new SD.fromRanges(ranges)
var jcamp=spectrum1H.toJcamp();

This could be equivalent to:

var spectrum = SD.fromMolfile(molfile, {
  kind: '1h'
}
var jcamp = spectrum.toJcamp();

@andcastillo
Copy link
Contributor

Then what is the difference between getRanges and createRanges? It would be better just to quit getRanges?

@lpatiny
Copy link
Member

lpatiny commented Dec 6, 2016

Ranges should be (is) stored in a Ranges class that extends array.
We may load the jcamp to create a SD and then setRanges based on what is stored in the database for example.
So getRanges should return current ranges and not recalculate them.
createRanges would indeed make the peakpicking and generate new ranges.
Did I miss something ?

@stropitek
Copy link
Contributor Author

stropitek commented Dec 6, 2016

Why does SD.predict1H return ranges and not peaks? Ranges are just a higher level of abstraction of peaks, so peaks are more general so to say. Then createRanges would create the ranges from peaks if they exist (for example when using the object was constructed from SD.fromPeaks), otherwise it would compute peaks from the spectra data and then create ranges from it.

@andcastillo
Copy link
Contributor

The ranges have a chemical shift, integration (or number of hydrogens), and a list of couplings which can be referenced to other ranges, so you can describe a spin system with the ranges and you can reconstruct the spectrum through a simulation , but the peaks are just x,y positions. If we have a list of x,y peaks, we can calculate a sum of gaussians or lorentzians to reconstruct the spectrum.

@lpatiny
Copy link
Member

lpatiny commented Dec 6, 2016

You are right that Ranges are composed of Signals that themself are composed of peaks. Normally a prediction will return Signals that is definitely a more precise representation of the spectrum.
However often it is an unrealistic representation because it yield to some integration of superimposed signals that can not be distinguished.
So for sure the simulation should be done fromSignals. So you are right it is better to have the prediction tool generate Signals !
Which also make more sense because if I remember well when we create 2D zones they are also composed of Signals.

@andcastillo
Copy link
Contributor

That is true. Daniel was confused with peaks/signals and I was confused with ranges/signals

@stropitek
Copy link
Contributor Author

stropitek commented Dec 6, 2016

Then I guess we need peaks2Signals and signalsToRanges to reflect that hierarchy (instead of just having peaks2Ranges like we do now

@lpatiny
Copy link
Member

lpatiny commented Dec 6, 2016

We could but it is not that simple because when spectra are not simulated but don't really know where are the 'real' signals.
When an auto peakpicking is done the result is that basically we have one signal that corresponds to one range. A human or in the future a smarter algorithm maybe could still split this range in many signals.
But indeed it could be something more general.

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

No branches or pull requests

3 participants