Skip to content

Commit 318e13f

Browse files
francoisbonneaufrancoisbonneau
committed
review
1 parent ed2d138 commit 318e13f

5 files changed

Lines changed: 62 additions & 62 deletions

File tree

‎include/geode/stochastic/sampling/mcmc/models/components/density_term.hpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@ namespace geode
3636
explicit DensityTerm( std::string_view name,
3737
double lambda,
3838
absl::flat_hash_set< uuid > targeted_subset_ids )
39-
: EnergyTerm< ObjectType >( name, lambda, targeted_subset_ids )
39+
: EnergyTerm< ObjectType >(
40+
name, lambda, std::move( targeted_subset_ids ) )
4041
{
4142
}
4243

‎include/geode/stochastic/sampling/mcmc/models/components/energy_term.hpp‎

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,13 @@
2323
#pragma once
2424

2525
#include <absl/container/flat_hash_set.h>
26+
27+
#include <geode/basic/identifier.hpp>
28+
#include <geode/basic/identifier_builder.hpp>
29+
2630
#include <geode/stochastic/common.hpp>
2731
#include <geode/stochastic/spatial/object_set.hpp>
32+
2833
#include <optional>
2934

3035
namespace geode
@@ -94,30 +99,22 @@ namespace geode
9499
// };
95100

96101
template < typename ObjectType >
97-
class EnergyTerm
102+
class EnergyTerm : public Identifier
98103
{
99104
public:
100105
explicit EnergyTerm( std::string_view name,
101106
double param,
102-
absl::flat_hash_set< uuid > targeted_subset_ids )
103-
: name_{ name },
107+
absl::flat_hash_set< uuid >&& targeted_subset_ids )
108+
: Identifier{},
104109
energy_scale_{ param },
105110
targeted_subset_ids_{ std::move( targeted_subset_ids ) }
106111
{
112+
IdentifierBuilder builder( *this );
113+
builder.set_name( name );
107114
}
108115

109116
virtual ~EnergyTerm() = default;
110117

111-
const uuid& id() const
112-
{
113-
return energy_term_id_;
114-
}
115-
116-
std::string_view name() const
117-
{
118-
return name_;
119-
}
120-
121118
double parameter() const
122119
{
123120
return energy_scale_.parameter();
@@ -186,10 +183,7 @@ namespace geode
186183
}
187184

188185
private:
189-
std::string name_;
190186
detail::EnergyScale energy_scale_;
191-
192187
absl::flat_hash_set< uuid > targeted_subset_ids_;
193-
uuid energy_term_id_{};
194188
};
195189
} // namespace geode

‎include/geode/stochastic/sampling/mcmc/models/components/energy_term_collection.hpp‎

Lines changed: 40 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,13 @@ namespace geode
1515
EnergyTermCollection( EnergyTermCollection&& ) noexcept = default;
1616
~EnergyTermCollection() = default;
1717

18-
uuid add_energy_term( std::unique_ptr< EnergyTerm< ObjectType > > term )
18+
uuid add_energy_term( std::shared_ptr< EnergyTerm< ObjectType > > term )
1919
{
2020
const uuid id = term->id();
21-
EnergyTerm< ObjectType >* ptr = term.get();
22-
23-
energy_terms_.emplace( id, std::move( term ) );
24-
25-
for( const uuid& subset_id : ptr->targeted_subset_ids() )
21+
energy_terms_.emplace( id, term );
22+
for( const uuid& subset_id : term->targeted_subset_ids() )
2623
{
27-
subset_to_terms_[subset_id].push_back( ptr );
24+
subset_to_terms_[subset_id].push_back( term );
2825
}
2926
return id;
3027
}
@@ -35,14 +32,20 @@ namespace geode
3532
if( it == energy_terms_.end() )
3633
return false;
3734

38-
EnergyTerm< ObjectType >* ptr = it->second.get();
39-
for( const uuid& subset_id : ptr->targeted_subset_ids() )
35+
auto term = it->second;
36+
37+
for( const uuid& subset_id : term->targeted_subset_ids() )
4038
{
41-
auto& vec = subset_to_terms_[subset_id];
39+
auto vec_it = subset_to_terms_.find( subset_id );
40+
if( vec_it == subset_to_terms_.end() )
41+
continue;
42+
43+
auto& vec = vec_it->second;
4244
vec.erase(
43-
std::remove( vec.begin(), vec.end(), ptr ), vec.end() );
45+
std::remove( vec.begin(), vec.end(), term ), vec.end() );
46+
4447
if( vec.empty() )
45-
subset_to_terms_.erase( subset_id );
48+
subset_to_terms_.erase( vec_it );
4649
}
4750

4851
energy_terms_.erase( it );
@@ -60,51 +63,54 @@ namespace geode
6063
return energy_terms_.size();
6164
}
6265

63-
[[nodiscard]] EnergyTerm< ObjectType >* get( const uuid& id ) const
66+
[[nodiscard]] std::shared_ptr< const EnergyTerm< ObjectType > > get(
67+
const uuid& id ) const
6468
{
6569
auto it = energy_terms_.find( id );
6670
OPENGEODE_EXCEPTION( it != energy_terms_.end(),
6771
absl::StrCat( "[EnergyTermCollection] Unknown energy term: ",
6872
id.string() ) );
69-
return it->second.get();
73+
return it->second;
7074
}
7175

72-
[[nodiscard]] std::vector< EnergyTerm< ObjectType >* > terms_for_subset(
73-
const uuid& subset_id ) const
76+
[[nodiscard]] const absl::flat_hash_map< uuid,
77+
std::shared_ptr< EnergyTerm< ObjectType > > >&
78+
all_terms() const
7479
{
75-
auto it = subset_to_terms_.find( subset_id );
76-
return it != subset_to_terms_.end()
77-
? it->second
78-
: std::vector< EnergyTerm< ObjectType >* >{};
80+
return energy_terms_;
7981
}
8082

81-
[[nodiscard]] std::vector< EnergyTerm< ObjectType >* > all_terms() const
83+
[[nodiscard]] const std::vector<
84+
std::shared_ptr< EnergyTerm< ObjectType > > >&
85+
terms_for_subset( const uuid& subset_id ) const
8286
{
83-
std::vector< EnergyTerm< ObjectType >* > result;
84-
result.reserve( energy_terms_.size() );
85-
for( const auto& [id, term] : energy_terms_ )
86-
{
87-
result.push_back( term.get() );
88-
}
89-
return result;
87+
const auto it = subset_to_terms_.find( subset_id );
88+
OPENGEODE_EXCEPTION( it != subset_to_terms_.end(),
89+
"[EnergyTermCollection] - Object Subset (", subset_id.string(),
90+
") does not have any energy term." );
91+
return it->second;
9092
}
9193

9294
std::string string() const
9395
{
9496
auto message = absl::StrCat(
95-
"Gibbs Energy: ", energy_terms_.size(), " terms:" );
96-
// we should find a way to iterate in a ordered way.
97-
for( const auto energy_term : all_terms() )
97+
"EnergyTermCollection: ", energy_terms_.size(), " terms:" );
98+
for( const auto& [id, term] : energy_terms_ )
9899
{
99-
absl::StrAppend( &message, "\n\t --> ", energy_term->string() );
100+
absl::StrAppend( &message, "\n\t --> ", term->string() );
100101
}
101102
return message;
102103
}
103104

104105
private:
105-
absl::flat_hash_map< uuid, std::unique_ptr< EnergyTerm< ObjectType > > >
106+
// strong ownership
107+
absl::flat_hash_map< uuid, std::shared_ptr< EnergyTerm< ObjectType > > >
106108
energy_terms_;
107-
absl::flat_hash_map< uuid, std::vector< EnergyTerm< ObjectType >* > >
109+
110+
// subset index (shared ownership)
111+
absl::flat_hash_map< uuid,
112+
std::vector< std::shared_ptr< EnergyTerm< ObjectType > > > >
108113
subset_to_terms_;
109114
};
115+
110116
} // namespace geode

‎include/geode/stochastic/sampling/mcmc/models/components/pairwise_term.hpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ namespace geode
3939
double gamma,
4040
absl::flat_hash_set< uuid > targeted_subset_ids,
4141
std::unique_ptr< PairwiseInteraction< ObjectType > > interaction )
42-
: EnergyTerm< ObjectType >( name, gamma, targeted_subset_ids ),
42+
: EnergyTerm< ObjectType >(
43+
name, gamma, std::move( targeted_subset_ids ) ),
4344
interaction_( std::move( interaction ) )
4445
{
4546
}

‎include/geode/stochastic/sampling/mcmc/models/gibbs_energy.hpp‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,38 +37,36 @@ namespace geode
3737
{
3838
}
3939

40-
// --- Total energy computation ---
4140
double total_log_energy( const ObjectSet< ObjectType >& state ) const
4241
{
4342
double log_energy = 0.0;
44-
for( auto* term : energy_terms_collection_.all_terms() )
43+
const auto& energy_terms = energy_terms_collection_.all_terms();
44+
for( auto& [id, term] : energy_terms )
4545
{
4646
log_energy += term->total_log( state );
4747
}
4848
return log_energy;
4949
}
5050

51-
// --- Subset-specific energy computation ---
5251
double total_log_energy_for_subset(
5352
const ObjectSet< ObjectType >& state, const uuid& subset_id ) const
5453
{
5554
double log_energy = 0.0;
56-
for( auto* term :
55+
for( const auto term :
5756
energy_terms_collection_.terms_for_subset( subset_id ) )
5857
{
5958
log_energy += term->total_log( state );
6059
}
6160
return log_energy;
6261
}
6362

64-
// --- Incremental updates ---
6563
double delta_log_add( const ObjectSet< ObjectType >& state,
6664
const ObjectRef< ObjectType >& new_object ) const
6765
{
6866
double log_energy = 0.0;
69-
for( const auto* term :
70-
energy_terms_collection_.terms_for_subset( new_object
71-
.subset ) ) // energy_terms_collection_.all_terms() )
67+
for( const auto& term : energy_terms_collection_.terms_for_subset(
68+
new_object
69+
.subset ) ) // energy_terms_collection_.all_terms() )
7270
{
7371
log_energy += term->delta_log_add( state, new_object );
7472
}
@@ -79,7 +77,7 @@ namespace geode
7977
const ObjectSet< ObjectType >& state, const ObjectId& id ) const
8078
{
8179
double log_energy = 0.0;
82-
for( const auto* term : energy_terms_collection_.terms_for_subset(
80+
for( const auto& term : energy_terms_collection_.terms_for_subset(
8381
id.subset ) ) // energy_terms_collection_.all_terms() )
8482
{
8583
log_energy += term->delta_log_remove( state, id );
@@ -92,7 +90,7 @@ namespace geode
9290
const ObjectRef< ObjectType >& new_object ) const
9391
{
9492
double log_energy = 0.0;
95-
for( const auto* term : energy_terms_collection_.terms_for_subset(
93+
for( const auto& term : energy_terms_collection_.terms_for_subset(
9694
old_id.subset ) ) // energy_terms_collection_.all_terms() )
9795
{
9896
log_energy +=

0 commit comments

Comments
 (0)