Skip to content

Add diver 1.3#533

Merged
anderkve merged 5 commits intomasterfrom
add_diver_1.3
Apr 7, 2025
Merged

Add diver 1.3#533
anderkve merged 5 commits intomasterfrom
add_diver_1.3

Conversation

@patscott
Copy link
Member

@patscott patscott commented Apr 5, 2025

This MR adds an interface to Diver 1.3.0.

This includes adding a new yaml option to the Diver scanner to allow the user to pass initial guesses to the scanner. The new option is now demonstrated in spartan.yaml

Also included in this MR is a new getSetParameters virtual function in the ScannerBit::BasePrior class, for checking precisely which parameters priors set, and therefore which parameters need to be specified in order to use their inverse_transform method successfully. For many priors this will just be the same as getParameters, but for the composite prior in particular things get more complicated, and it becomes necessary to differentiate between

  • parameters directly set by the prior in the course of a scan, and therefore needed for any application of the inverse transform (getSetParameters),
  • those whose sub-priors contribute to the definition of the composite prior (getParameters), and
  • the effective parameters reported by the prior as being varied in the course of a scan (getShownParameters).

The difference between these is not important for the example in this PR, but it is crucial for the example in the matching GAMBIT_light PR.

patscott added 2 commits April 5, 2025 23:04
…sses in spartan.yaml

	new file:   ScannerBit/include/gambit/ScannerBit/scanners/diver_1.3.0/diver.hpp
	new file:   ScannerBit/src/scanners/diver_1.3.0/diver.cpp
	modified:   cmake/scanners.cmake
	modified:   config/scanner_locations.yaml.default
	modified:   yaml_files/spartan.yaml
	modified:   ScannerBit/src/scanners/diver_1.3.0/diver.cpp
@patscott patscott requested a review from anderkve April 5, 2025 14:20
	modified:   ScannerBit/src/scanners/diver_1.3.0/diver.cpp
patscott and others added 2 commits April 6, 2025 13:20
…hich parameters they set,

and therefore which parameters need to be specified in order to use theire inverse_transform method.
Also updated Diver 1.3 interface to use getSetParameters as a more robust way of checking that the
parameters provided by the user as initial guesses are the same as those being scanned.
	modified:   ScannerBit/include/gambit/ScannerBit/base_prior.hpp
	modified:   ScannerBit/include/gambit/ScannerBit/priors/composite.hpp
	modified:   ScannerBit/src/priors/composite.cpp
	modified:   ScannerBit/src/scanners/diver_1.3.0/diver.cpp
Copy link
Collaborator

@anderkve anderkve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@anderkve
Copy link
Collaborator

anderkve commented Apr 6, 2025

Thanks for doing this so quickly, @patscott! Looks good to me, and works as expected in the tests I've done.

If I understand the complication with thecomposite prior correctly, we will need to use the getSetParameters() method also for Python scanners that need to do some inverse transforms. So I added a new propery set_parameter_names to the Python module we use for the Python scanner plugins. This propery is filled by getSetParameters in the same way as the existing property parameter_names is filled by getShownParameters(). See here:

.def_property_readonly_static("set_parameter_names", [](py::object)
{
static py::list names = scanner_base::to_list<std::string>(get_prior().getSetParameters());
return names;
})

I'll hold off merging this until you've had a chance to give a thumbs up/down for that addition.

(The follow-up task of testing how the various current Python scanners currently work/fail with the composite prior I propose we tackle as a separate issue, to not delay this PR.)

@patscott
Copy link
Member Author

patscott commented Apr 7, 2025

Perfect, thanks!

@anderkve anderkve merged commit 2abfd07 into master Apr 7, 2025
2 of 11 checks passed
@anderkve anderkve deleted the add_diver_1.3 branch April 7, 2025 06:13
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

Successfully merging this pull request may close these issues.

2 participants