From 5b2dba6a64cfc696d291add1c017dcba2e342659 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-S=C3=A9bastien=20Caux?= <git@jscaux.org>
Date: Sun, 17 Mar 2024 08:43:31 +0100
Subject: [PATCH] Increase accuracy of PubFrac value calculations and display
---
scipost_django/finances/allocate.py | 1 -
...lue.py => recalculate_pubfrac_cf_value.py} | 7 +-----
.../migrations/0043_alter_pubfrac_cf_value.py | 20 +++++++++++++++
scipost_django/finances/models/pubfrac.py | 25 +++++++------------
scipost_django/finances/models/subsidy.py | 14 +++++++++++
.../templates/finances/_subsidy_details.html | 23 +++++++++--------
scipost_django/journals/models/publication.py | 11 ++++----
.../journals/publication_detail.html | 8 +++---
8 files changed, 66 insertions(+), 43 deletions(-)
rename scipost_django/finances/management/commands/{recompute_pubfrac_cf_value.py => recalculate_pubfrac_cf_value.py} (63%)
create mode 100644 scipost_django/finances/migrations/0043_alter_pubfrac_cf_value.py
diff --git a/scipost_django/finances/allocate.py b/scipost_django/finances/allocate.py
index 965f2ef90..6284cfeb3 100644
--- a/scipost_django/finances/allocate.py
+++ b/scipost_django/finances/allocate.py
@@ -67,7 +67,6 @@ def allocate_to_all_aff(subsidy: Subsidy):
publication__doi_label=pubfrac.publication.doi_label,
compensated_by__isnull=True,
)
- print(f"{pubfracs_for_pub.all() = }")
for pf in pubfracs_for_pub.all():
if pf.cf_value <= subsidy.remainder:
pf.compensated_by = subsidy
diff --git a/scipost_django/finances/management/commands/recompute_pubfrac_cf_value.py b/scipost_django/finances/management/commands/recalculate_pubfrac_cf_value.py
similarity index 63%
rename from scipost_django/finances/management/commands/recompute_pubfrac_cf_value.py
rename to scipost_django/finances/management/commands/recalculate_pubfrac_cf_value.py
index 957ffd7de..7b00c1c28 100644
--- a/scipost_django/finances/management/commands/recompute_pubfrac_cf_value.py
+++ b/scipost_django/finances/management/commands/recalculate_pubfrac_cf_value.py
@@ -12,10 +12,5 @@ class Command(BaseCommand):
def handle(self, *args, **kwargs):
for pf in PubFrac.objects.all():
- pf.cf_value = int(
- pf.fraction
- * pf.publication.get_journal().cost_per_publication(
- pf.publication.publication_date.year
- )
- )
+ pf.cf_value = 0 # just trigger recomputation via the save method
pf.save()
diff --git a/scipost_django/finances/migrations/0043_alter_pubfrac_cf_value.py b/scipost_django/finances/migrations/0043_alter_pubfrac_cf_value.py
new file mode 100644
index 000000000..143982ba2
--- /dev/null
+++ b/scipost_django/finances/migrations/0043_alter_pubfrac_cf_value.py
@@ -0,0 +1,20 @@
+# Generated by Django 4.2.10 on 2024-03-17 06:37
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ("finances", "0042_subsidy_algorithm_subsidy_algorithm_data"),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name="pubfrac",
+ name="cf_value",
+ field=models.DecimalField(
+ blank=True, decimal_places=3, max_digits=16, null=True
+ ),
+ ),
+ ]
diff --git a/scipost_django/finances/models/pubfrac.py b/scipost_django/finances/models/pubfrac.py
index 3cd0f66db..d347408e7 100644
--- a/scipost_django/finances/models/pubfrac.py
+++ b/scipost_django/finances/models/pubfrac.py
@@ -5,13 +5,11 @@ __license__ = "AGPL v3"
from decimal import Decimal
from django.db import models
-from django.db.models.signals import pre_save
-from django.dispatch import receiver
class PubFrac(models.Model):
"""
- A fraction of a given Publication related to an Organization, for expenditure redistribution.
+ A fraction of a given Publication related to an Organization.
Fractions for a given Publication should sum up to one.
@@ -41,7 +39,9 @@ class PubFrac(models.Model):
)
# Calculated field
- cf_value = models.PositiveIntegerField(blank=True, null=True)
+ cf_value = models.DecimalField(
+ max_digits=16, decimal_places=3, blank=True, null=True
+ )
class Meta:
unique_together = (("organization", "publication"),)
@@ -50,21 +50,14 @@ class PubFrac(models.Model):
def __str__(self):
return (
- f"{str(self.fraction)} (€{self.cf_value}) "
+ f"{str(self.fraction)} (€{int(self.cf_value)}) "
f"for {self.publication.doi_label} from {self.organization}"
)
+ def save(self, *args, **kwargs):
+ self.cf_value = self.fraction * self.publication.expenditures
+ return super().save(*args, **kwargs)
+
@property
def compensated(self):
return self.compensated_by is not None
-
-
-@receiver(pre_save, sender=PubFrac)
-def calculate_cf_value(sender, instance: PubFrac, **kwargs):
- """Calculate the cf_value field before saving."""
- instance.cf_value = int(
- instance.fraction
- * instance.publication.get_journal().cost_per_publication(
- instance.publication.publication_date.year
- )
- )
diff --git a/scipost_django/finances/models/subsidy.py b/scipost_django/finances/models/subsidy.py
index 2ad331eb5..fb42e1354 100644
--- a/scipost_django/finances/models/subsidy.py
+++ b/scipost_django/finances/models/subsidy.py
@@ -2,6 +2,7 @@ __copyright__ = "Copyright © Stichting SciPost (SciPost Foundation)"
__license__ = "AGPL v3"
+from collections import OrderedDict
import datetime
from django.db import models
@@ -207,3 +208,16 @@ class Subsidy(models.Model):
Part of the Subsidy amount which hasn't been allocated.
"""
return self.amount - self.total_compensations
+
+ @property
+ def compensated_pubfracs_dict(self):
+ """A dict containing digested info on compensated PubFracs."""
+ compensated = OrderedDict.fromkeys(
+ [pf.publication.doi_label for pf in self.compensated_pubfracs.all()]
+ )
+ for pubfrac in self.compensated_pubfracs.all():
+ compensated[pubfrac.publication.doi_label] = {"fraction": 0, "value": 0}
+ for pubfrac in self.compensated_pubfracs.all():
+ compensated[pubfrac.publication.doi_label]["fraction"] += pubfrac.fraction
+ compensated[pubfrac.publication.doi_label]["value"] += pubfrac.cf_value
+ return compensated
diff --git a/scipost_django/finances/templates/finances/_subsidy_details.html b/scipost_django/finances/templates/finances/_subsidy_details.html
index 04f7c3965..1c3df29fb 100644
--- a/scipost_django/finances/templates/finances/_subsidy_details.html
+++ b/scipost_django/finances/templates/finances/_subsidy_details.html
@@ -123,29 +123,32 @@
<div class="row">
<div class="col-lg-6">
<table class="table mt-2 caption-top">
- <caption>PubFrac Compensations</caption>
+ <caption>PubFrac compensations</caption>
<thead class="table-light">
<tr>
<th>Publication</th>
- <th>PubFrac value</th>
+ <th>PubFrac</th>
+ <th>value</th>
</tr>
</thead>
<tbody>
- {% for compensated_pubfrac in subsidy.compensated_pubfracs.all %}
+ {% for doi_label, pf in subsidy.compensated_pubfracs_dict.items %}
<tr>
<td>
- <a href="{% url 'scipost:publication_detail' doi_label=compensated_pubfrac.publication.doi_label %}">{{ compensated_pubfrac.publication.doi_label }}</a>
+ <a href="{% url 'scipost:publication_detail' doi_label=doi_label %}">{{ doi_label }}</a>
</td>
- <td>€{{ compensated_pubfrac.cf_value }}</td>
+ <td>{{ pf.fraction }}</td>
+ <td>€{{ pf.value|floatformat:0 }}</td>
</tr>
{% empty %}
<tr>
- <td>No Compensation defined</td>
+ <td>No compensation found</td>
</tr>
{% endfor %}
<tr class="bg-secondary bg-opacity-10">
<th>Total compensations from this Subsidy</th>
- <td>€{{ subsidy.total_compensations }}</td>
+ <td></td>
+ <td>€{{ subsidy.total_compensations|floatformat:0 }}</td>
</tr>
</tbody>
</table>
@@ -160,11 +163,11 @@
</tr>
<tr>
<th>PubFrac Compensations</th>
- <td>€{{ subsidy.total_compensations }}</td>
+ <td>€{{ subsidy.total_compensations|floatformat:0 }}</td>
</tr>
<tr class="bg-secondary bg-opacity-10">
- <th>Remainder (allocated to reserve fund)</th>
- <td>€{{ subsidy.remainder }}</td>
+ <th>Remainder (allocated to our reserve fund)</th>
+ <td>€{{ subsidy.remainder|floatformat:0 }}</td>
</tr>
</tbody>
</table>
diff --git a/scipost_django/journals/models/publication.py b/scipost_django/journals/models/publication.py
index 298c0318d..72547530e 100644
--- a/scipost_django/journals/models/publication.py
+++ b/scipost_django/journals/models/publication.py
@@ -442,14 +442,13 @@ class Publication(models.Model):
for aff in author.affiliations.all():
fraction[aff.id] += 1.0 / (nr_authors * nr_affiliations)
for org in affiliations.all():
- value = int(fraction[org.id] * self.expenditures)
- PubFrac.objects.filter(
+ pubfrac, created = PubFrac.objects.get_or_create(
publication=self,
organization=org,
- ).update(
- fraction=fraction[org.id],
- cf_value=value,
)
+ # ensure 3 digit accuracy for all fractions through integer cast
+ pubfrac.fraction = 0.001 * int(1000 * fraction[org.id])
+ pubfrac.save()
self.ensure_pubfracs_sum_to_1()
def ensure_pubfracs_sum_to_1(self):
@@ -469,7 +468,7 @@ class Publication(models.Model):
"""Compensated part of expenditures for this Publication."""
qs = self.pubfracs.filter(compensated_by__isnull=False)
# Use the fraction to obtain an accurate result
- return int(
+ return (
qs.aggregate(Sum("fraction"))["fraction__sum"] * self.expenditures
if qs.exists()
else 0
diff --git a/scipost_django/journals/templates/journals/publication_detail.html b/scipost_django/journals/templates/journals/publication_detail.html
index 6b1d9dff5..bcd95c126 100644
--- a/scipost_django/journals/templates/journals/publication_detail.html
+++ b/scipost_django/journals/templates/journals/publication_detail.html
@@ -189,7 +189,7 @@
<tr>
<td><a href="{% url 'organizations:organization_detail' pk=pubfrac.organization.id %}">{{ pubfrac.organization }}</a></td>
<td>{{ pubfrac.fraction }}</td>
- <td>€{{ pubfrac.cf_value }}</td>
+ <td>€{{ pubfrac.cf_value|floatformat:0 }}</td>
{% if pubfrac.compensated %}
<td class="bg-success bg-opacity-25">
by <a href="{% url 'organizations:organization_detail' pk=pubfrac.compensated_by.organization.id %}">{{ pubfrac.compensated_by.organization }}</a>
@@ -202,7 +202,7 @@
<td class="bg-danger bg-opacity-25">
<span class="text-danger">{% include 'bi/x-circle-fill.html' %}</span>
</td>
- <td class="bg-danger bg-opacity-25">€{{ pubfrac.cf_value }}</td>
+ <td class="bg-danger bg-opacity-25">€{{ pubfrac.cf_value|floatformat:0 }}</td>
{% endif %}
</tr>
{% endfor %}
@@ -211,8 +211,8 @@
<th>Totals</th>
<td>1</td>
<td>€{{ publication.expenditures }}</td>
- <td class="{% if uncompensated == 0 %}bg-success{% else %}bg-danger{% endif %} bg-opacity-25">€{{ publication.compensated_expenditures }}</td>
- <td class="{% if uncompensated == 0 %}bg-success{% else %}bg-danger{% endif %} bg-opacity-25">€{{ publication.uncompensated_expenditures }}</td>
+ <td class="{% if uncompensated == 0 %}bg-success{% else %}bg-danger{% endif %} bg-opacity-25">€{{ publication.compensated_expenditures|floatformat:0 }}</td>
+ <td class="{% if uncompensated == 0 %}bg-success{% else %}bg-danger{% endif %} bg-opacity-25">€{{ publication.uncompensated_expenditures|floatformat:0 }}</td>
</tr>
{% endwith %}
</tbody>
--
GitLab