[REDMINE1D-327] [RM-8382] [parameters checker] wrong check for velocitifit in linemeasSolve Created: 05/Oct/23  Updated: 07/Nov/23  Resolved: 07/Nov/23

Status: Done
Project: 1D Redmine
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Normal
Reporter: Redmine-Jira Migtation Assignee: Redmine-Jira Migtation
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Created on 2023-10-04 16:26:25 by Didier Vibert. % Done: 100

MR pylibamazed: https://gitlab.lam.fr/CPF/cpf-redshift/-/merge_requests/558
MR dataset-parameters: https://gitlab.lam.fr/amazed/dataset-parameters/-/merge_requests/65

when linemeasSolve and fittingmethod is "lbfgs" the velocityfit parameter is required.
and the checker is issuing a warning:

<pre>
Warning: Unused parameter object qso LineMeasSolve velocityfit
</pre>

the check is wrong because @"lbfgsb"@ is used for the ftting method instead of @"lbfgs"@ :

<pre><code class="python">
def _check_linemeassolve_fittingmethod_lbfgsb_velocityfit(self, object_type: str):
self._check_dependant_parameter_presence(
self.accessor.get_lineMeasSolve_fittingmethod(object_type) == "lbfgsb",
self.accessor.get_lineMeasSolve_velocityfit(object_type) is not None,
error_message=f"LineMeasSolve velocityfit for object

{object_type}",
warning_message=f"object {object_type}

LineMeasSolve velocityfit"
)
</code></pre>

 Comments   
Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Didier Vibert on 2023-10-04 16:27:19:
added @fdufresne as watcher

Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Fanny Dufresne on 2023-10-12 16:06:58:
je vois lbfgsb dans

  • la doc parameters
  • le json schema
  • ce check
  • tout le python

et lbfgs dans:

  • le c++

Si je comprends bien la modif à faire est un petit peu plus profonde que juste le check, il faudrait modifier toutes les occurences dans le python ou bien le c ++ ?

@dvibert tu peux me confirmer que c'est bien ça ?

Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Didier Vibert on 2023-10-12 17:05:46:
oui bien vu je confirme,

la librairie externe s'appelle "LBFGSpp" pour Limited-memory BFGS, BFGS pour Broyden–Fletcher–Goldfarb–Shanno du nom des auteurs de l'algo, et pp pour C++ !
Cette lib implémente en fait 2 algos:

  1. LBFGS
  2. LBFGSB . ce deuxième algo, en fait assez évolué par rapport au précédent, implémente une contrainte de type "box" sur les param (d'où le B final) et permet donc d'imposer une contrainte min,max sur tous les paramètres.

c'est bien LBFGSB qu'on utilise, justement pour cette capacité de contrainte, donc effectivement, si tu peux renommer dans le C+, le lbfgs en lbfgsb welcome (mais ne modifie pas les noms provenant de la lib externe LBFGSpp). Sinon, tu peux aussi te limiter à modifier le C+ uniquement pour prendre "lbfgsb" comme valeur du paramètre fittingmethod.

Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Fanny Dufresne on 2023-10-13 07:09:16:
Ok super, merci pour les explications ! je vais juste remplacer les apparitions du mot "lbfgs" par "lbfgsb"

C'est le CLbfgsFitter en particulier que tu proposes de renommer en CLbfgsbFitter ?

Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Didier Vibert on 2023-10-19 09:08:15:
il y a aussi des modifs à faire sur dataset-parameters (variation linemeas_lbfgsb.json, ...)
peut être faire un grep lbfgs sur tous les json parameters

c'est étonnant d'ailleurs, les IT sont passés sans changer linemeas_lbfgsb.json ?

Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Fanny Dufresne on 2023-10-19 14:59:38:
C'est modifié ! Il se pourrait que je n'ai pas fait passer tous les ITs auparavant ^^
Mais maintenant ils passent

Comment by Redmine-Jira Migtation [ 07/Nov/23 ]

Comment by Pierre-yves Chabaud on 2023-10-24 14:37:16:
Merged into @develop@ (@b391592c@)

Generated at Sat Feb 10 15:31:36 JST 2024 using Jira 8.3.4#803005-sha1:1f96e09b3c60279a408a2ae47be3c745f571388b.