search

gSulpizio / isotherm-fitting

MIT License
1 stars 1 forks source link

some comments #1

Open kjappelbaum opened 3 years ago

kjappelbaum commented 3 years ago
  1. https://github.com/gSulpizio/isotherm-analysis/blob/7ff9243f06472539a53a28af52af709bd9856a81/src/BETFitWeighted.ts#L12

Du you want the first weight to always be 1? According to Fig 2 in the paper it is not light that in the original implementation. You might just take two points that you would not consider for the fit (w=0) at the boundaries

did you console.log the weights? How are they distributed? Did you try to set them uniformly for debugging?

please also note https://github.com/pauliacomi/pyGAPS/blob/69f0037aa8a64b49f0e5b036a83d67f463e5743f/src/pygaps/characterisation/area_bet.py#L320-L345

  1. I would add docstrings with reference to the original paper and equation, in this case eq. 11. You would at some point also need to think about how you want to deal with the units. Or at least give the user a clue what dimensionality the result will have.

  2. https://github.com/gSulpizio/isotherm-analysis/blob/7ff9243f06472539a53a28af52af709bd9856a81/src/modelFunctions.ts#L19 what do you mean with the comment? When shouldn't p be a number?

gSulpizio commented 3 years ago

So the weights are around 1 (+/-, with some variation after the 5th decimal) except for the 2 first numbers that are around 2 and 1.1. The first and last weight have been set to 0. setting them uniformly for debuggng did not really help.

For 3, indeed, just it was weird that it threw an error if the type is any, no?

kjappelbaum commented 3 years ago

But setting all weights to 1 is per definition just simple plain normal linear regression which used to work for you

On May 13, 2021 at 01:13, gSulpizio @.**@.>> wrote:

So the weights are around 1 (+/-, with some variation after the 5th decimal) except for the 2 first numbers that are around 2 and 1.1. The first and last weight have been set to 0. setting them uniformly for debuggng did not really help.

For 3, indeed, just it was weird that it threw an error if the type is any, no?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/gSulpizio/isotherm-analysis/issues/1#issuecomment-840152064, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AH3I2QIS4N4SU43G3J3IPWTTNMDP5ANCNFSM432I5KZQ.