smeerten / ssnake

A program for the analysis of NMR data.
Other
19 stars 15 forks source link

Fitting: changing CSA definition clashes with Connect Parameter #112

Open wfranssen opened 2 years ago

wfranssen commented 2 years ago

After connecting a CSA parameter, changing CSA definition is still allowed, but leads to an error. Moreover, what should happen if the value it connects to changes definition?

Possible fix could be to block definition change if a Connect Parameter is active somewhere. And additionally allow only one definition shared by all data tabs? (Traces are handled in this way already.)

When forcing one definition for all data, we might restrict the user in linking parameters. I cannot think of a good reason to connect parameters from different CSA definitions. But who knows...

The issue of course occurs for all fitting routines where definition changes are possible.

jtrebosc commented 2 years ago

In my opinion, 1) one should use the same CSA definition for all fitted data tabs. 2) CSA definition change can be made but conversion must be made for all data tabs 3) connected parameters are the responsability of the user as long as definition1 connects to definition1 and so on. iso/d_aniso/eta to/from iso/omega/skew is not a problem. Changing to d_iso to/from deltaxx a connected parameter may not be the best choice but will not result in inconsistent simulation.

wfranssen commented 1 year ago

Looking again at this bug. My current preferred solution is:

This at least fixes the issue for within a 'tab'.

Linking between tabs leaves some problems that are the users problem. If we change convention in tab1, links from tab2 to tab1 change meaning, which is a 'user problem'. If we have links from tab1 to tab2, we could in principle use the linked value to allow convention definition changes in tab1, but this will be very unclear for users. Hence my suggestion to not allow the convention change in this case.

jtrebosc commented 1 year ago

Hi Wouter, Since I'm working on a new class based model, this bug would not be relevant or cases should be discussed with class design. However it is a long way before a class model would replace current implementation. Hence we should look for a quick fix. I'll send an invitation for presenting my initial work on model classes somewhere 10th to 15th of June.